Skip to content

Commit

Permalink
Clear out CreateReactInstanceTaskRef early when destroying (#46544)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #46544

Experiments with `completeReactInstanceCreationOnBgThreadOnAndroid` have shown a native crash in this code path, but it also existed previously.

I believe this is due to the ordering of ReactInstance teardown: we first destroy the ReactInstance, and only then destroy the reference to it in mCreateReactInstanceTaskRef. When using the immediate executor and completeReactInstanceCreationOnBgThreadOnAndroid, we read the react instance from the task, which may no longer be valid at this point. Resetting the task at the earliest point should mitigate the issue.

Changelog: [Internal]

Reviewed By: markv

Differential Revision: D62872625

fbshipit-source-id: 5aaccd53433ca806d2b93f7e3dd2bcf9bf8c09a8
  • Loading branch information
javache authored and facebook-github-bot committed Sep 19, 2024
1 parent 6d51c5c commit d8aa664
Showing 1 changed file with 29 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1400,9 +1400,14 @@ private Task<ReactInstance> getOrCreateReloadTask(String reason) {
createReactInstanceUnwrapper("Reload", method, reason);

if (mReloadTask == null) {
// When using the immediate executor, we want to avoid scheduling any further work immediately
// when destruction is kicked off.
Task<ReactInstance> createTask =
ReactNativeFeatureFlags.completeReactInstanceCreationOnBgThreadOnAndroid()
? mCreateReactInstanceTaskRef.getAndReset()
: mCreateReactInstanceTaskRef.get();
mReloadTask =
mCreateReactInstanceTaskRef
.get()
createTask
.continueWithTask(
(task) -> {
log(method, "Starting React Native reload");
Expand Down Expand Up @@ -1499,8 +1504,13 @@ private Task<ReactInstance> getOrCreateReloadTask(String reason) {
reactInstance.destroy();
}

log(method, "Resetting createReactInstance task ref");
mCreateReactInstanceTaskRef.reset();
// Originally, we reset the instance task ref quite late, leading to potential
// racing invocations while shutting down
if (!ReactNativeFeatureFlags
.completeReactInstanceCreationOnBgThreadOnAndroid()) {
log(method, "Resetting createReactInstance task ref");
mCreateReactInstanceTaskRef.reset();
}

log(method, "Resetting start task ref");
mStartTask = null;
Expand Down Expand Up @@ -1578,9 +1588,15 @@ private Task<Void> getOrCreateDestroyTask(final String reason, @Nullable Excepti
createReactInstanceUnwrapper("Destroy", method, reason);

if (mDestroyTask == null) {
// When using the immediate executor, we want to avoid scheduling any further work immediately
// when destruction is kicked off.
Task<ReactInstance> createTask =
ReactNativeFeatureFlags.completeReactInstanceCreationOnBgThreadOnAndroid()
? mCreateReactInstanceTaskRef.getAndReset()
: mCreateReactInstanceTaskRef.get();

mDestroyTask =
mCreateReactInstanceTaskRef
.get()
createTask
.continueWithTask(
task -> {
log(method, "Starting React Native destruction");
Expand Down Expand Up @@ -1608,7 +1624,6 @@ private Task<Void> getOrCreateDestroyTask(final String reason, @Nullable Excepti
}

final ReactContext reactContext = mBridgelessReactContextRef.getNullable();

if (reactContext == null) {
raiseSoftException(method, "ReactContext is null. Destroy reason: " + reason);
}
Expand All @@ -1624,7 +1639,6 @@ private Task<Void> getOrCreateDestroyTask(final String reason, @Nullable Excepti
task -> {
final ReactInstance reactInstance =
reactInstanceTaskUnwrapper.unwrap(task, "2: Stopping surfaces");

if (reactInstance == null) {
raiseSoftException(method, "Skipping surface shutdown: ReactInstance null");
return task;
Expand Down Expand Up @@ -1701,8 +1715,13 @@ private Task<Void> getOrCreateDestroyTask(final String reason, @Nullable Excepti
reactInstance.destroy();
}

log(method, "Resetting createReactInstance task ref");
mCreateReactInstanceTaskRef.reset();
// Originally, we reset the instance task ref quite late, leading to potential
// racing invocations while shutting down
if (!ReactNativeFeatureFlags
.completeReactInstanceCreationOnBgThreadOnAndroid()) {
log(method, "Resetting createReactInstance task ref");
mCreateReactInstanceTaskRef.reset();
}

log(method, "Resetting start task ref");
mStartTask = null;
Expand Down

0 comments on commit d8aa664

Please sign in to comment.