Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix app crashing when app is not yet started on some devices #215

Merged
merged 16 commits into from
Nov 15, 2021

Conversation

ben-xD
Copy link
Contributor

@ben-xD ben-xD commented Nov 10, 2021

There are actually three bugs:

  • activityManager.getRunningAppProcesses() can return null when no processes are running. I was iterating over it, assuming it would be an empty list instead of null. This caused a NPE like crash. (3 line fix, excluding comments)
  • When the app is in terminated state, no messages are received. Solution: launch BackgroundIsolateAndroidPlatform. (one line fix)
  • More difficult to solve: When an app is in terminated state, and a message is received, subsequent messages are not received. Specifically, these subsequent messages do not arrive to the Dart side. This is because I was using a MethodChannel created when no activity existed. When the activity is created, a new FlutterEngine (and FlutterJNI) is created. I have to use this new environment instead of the old method channel (which used the old FlutterJNI that was detached). This is done in PushMessagingEventHandlers.load(applicationContext, methodChannel);
    • error: Tried to send a platform message to Flutter, but FlutterJNI was detached from native C++..

See customer report: https://ably-real-time.slack.com/archives/C012DA29VM0/p1636555532089600?thread_ts=1636544116.088600&cid=C012DA29VM0

@ben-xD ben-xD added the bug Something isn't working. It's clear that this does need to be fixed. label Nov 10, 2021
@ben-xD ben-xD self-assigned this Nov 10, 2021
@github-actions github-actions bot temporarily deployed to staging/pull/215/dartdoc November 10, 2021 17:42 Inactive
@ben-xD ben-xD marked this pull request as ready for review November 10, 2021 20:17
@ben-xD ben-xD changed the title Fix app crashing when app is not yet started on some devices 0 Fix app crashing when app is not yet started on some devices Nov 10, 2021
Copy link
Contributor

@ikbalkaya ikbalkaya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I just requested a comment removal

@ben-xD ben-xD force-pushed the bug/app-processes-is-null branch from a59d688 to 44b9b91 Compare November 11, 2021 11:55
@github-actions github-actions bot temporarily deployed to staging/pull/215/dartdoc November 11, 2021 11:59 Inactive
@ben-xD ben-xD requested a review from ikbalkaya November 11, 2021 12:01
@github-actions github-actions bot temporarily deployed to staging/pull/215/dartdoc November 11, 2021 13:30 Inactive
Copy link
Contributor

@ikbalkaya ikbalkaya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

… future messages received when app is running (in foreground or background) will fail. This was because of retaining an old FlutterJNI and methodChannel.
@github-actions github-actions bot temporarily deployed to staging/pull/215/dartdoc November 11, 2021 18:19 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/215/dartdoc November 12, 2021 08:03 Inactive
@ben-xD ben-xD requested a review from ikbalkaya November 12, 2021 09:42
@github-actions github-actions bot temporarily deployed to staging/pull/215/dartdoc November 12, 2021 09:56 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/215/dartdoc November 12, 2021 10:26 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/215/dartdoc November 12, 2021 10:40 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/215/dartdoc November 12, 2021 11:39 Inactive
@ben-xD ben-xD requested a review from tiholic November 12, 2021 13:32
Copy link
Contributor

@ikbalkaya ikbalkaya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall. I made some requests about some minor comment changes. There are also some places where nullable variables are accessed without any check or annotation.

@github-actions github-actions bot temporarily deployed to staging/pull/215/dartdoc November 12, 2021 15:29 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/215/dartdoc November 12, 2021 15:33 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/215/dartdoc November 12, 2021 15:41 Inactive
@ben-xD ben-xD requested a review from ikbalkaya November 12, 2021 15:51
Copy link
Contributor

@ikbalkaya ikbalkaya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good :shipit:

@ben-xD ben-xD merged commit 31c42f2 into main Nov 15, 2021
@ben-xD ben-xD deleted the bug/app-processes-is-null branch November 15, 2021 08:39
@ben-xD ben-xD changed the title 0 Fix app crashing when app is not yet started on some devices Fix app crashing when app is not yet started on some devices Nov 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working. It's clear that this does need to be fixed.
Development

Successfully merging this pull request may close these issues.

None yet

3 participants