fix(android): don't crash in Fragment.onCreateView on stale-fragment race#11264
fix(android): don't crash in Fragment.onCreateView on stale-fragment race#11264NathanWalker wants to merge 3 commits into
Conversation
…race onCreateView already returns null when its entry, resolvedPage, or frame is missing, but it first called Trace.error, which rethrows through the registered error handler across the JNI boundary and turns the recoverable case into a fatal "Calling js method onCreateView failed" crash. These nulls happen when the FragmentManager drives a stale fragment to CREATE_VIEW after NativeScript has already torn down the corresponding backstack entry (process death / activity recreation after rotation or theme change, clearHistory, or rapid navigate+goBack). The fragment should just be discarded, not crash the app. Downgrade the three guards to Trace.messageType.warn so the existing return-null recovery path is actually reached, consistent with the existing onDestroy handling of the same race.
|
View your CI Pipeline Execution ↗ for commit 26d34e1
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗ ☁️ Nx Cloud last updated this comment at |
| // Recoverable race: a stale fragment is being driven to onCreateView by the | ||
| // FragmentManager after its entry was cleared. | ||
| // Using Trace.error (routes to an error handler, which throws) would be fatal as the error handler rethrows across the JNI boundary. | ||
| if (Trace.isEnabled()) { |
There was a problem hiding this comment.
@NathanWalker We can't ignore the possibility that it may also occur if something goes wrong.
If it occurs too frequently in your apps, it's a sign there's something we shall take care of.
I'd suggest we removed the isEnabled check and use messageType.error instead to let message be printed no matter what.
There was a problem hiding this comment.
That would work as well, will adjust 👍 (main thing is to avoid the trace.error there)
There was a problem hiding this comment.
@copilot please remove the isEnabled checks around the Trace.write in this PR and switch to Trace.messageType.error for each.
There was a problem hiding this comment.
Updated in 2b51674: removed the Trace.isEnabled() guards around those Trace.write calls and switched all three to Trace.messageType.error. I also added comment clarifications in 11902bc to make clear this remains non-fatal.
onCreateView already returns null when its entry, resolvedPage, or frame is missing, but it first called Trace.error, which rethrows through the registered error handler across the JNI boundary and turns the recoverable case into a fatal "Calling js method onCreateView failed" crash.
These nulls happen when the FragmentManager drives a stale fragment to CREATE_VIEW after NativeScript has already torn down the corresponding backstack entry (process death / activity recreation after rotation or theme change, clearHistory, or rapid navigate+goBack). The fragment should just be discarded, not crash the app.
Replace those three Trace.error calls with Trace.write using Trace.messageType.error, and remove the Trace.isEnabled guards around those writes so the logs are always emitted while the existing return-null recovery path is still reached (consistent with existing onDestroy handling of the same race).
Fixes production crashlytics reports: