Skip to content

Fix null pointer crash in Scheduler::animationTick by adding null check for uiManager_ (#56128) (#56128)#57327

Closed
shubhamksavita wants to merge 1 commit into
mainfrom
export-D93363797
Closed

Fix null pointer crash in Scheduler::animationTick by adding null check for uiManager_ (#56128) (#56128)#57327
shubhamksavita wants to merge 1 commit into
mainfrom
export-D93363797

Conversation

@shubhamksavita

Copy link
Copy Markdown
Contributor

Summary:

Changelog: [Android] [Fixed] - Fix crash in Scheduler::animationTick when uiManager_ is null.

Test Plan:
CI

Fixes a null pointer dereference crash in Scheduler::animationTick() that occurs during shutdown race conditions.

Root Cause Analysis

The Symptom: uiManager_ is null when Scheduler::animationTick() is called, causing a crash at offset 0x50 from null when accessing members of UIManager.

The Root Cause: During shutdown, when uninstallFabricUIManager() is called, the Choreographer's animation frame callback (doFrame) can still arrive. While FabricUIManagerBinding::driveCxxAnimations() checks for null scheduler (added in D92986523), the internal Scheduler::animationTick() method didn't check if uiManager_ is valid before dereferencing it:

void Scheduler::animationTick() const {
  uiManager_->animationTick();  // No null check - crashes if uiManager_ is null
}

The Fix: Added a null check for uiManager_ before accessing it, following the same defensive pattern used in driveCxxAnimations() and other methods in the codebase:

void Scheduler::animationTick() const {
  if (!uiManager_) {
    return;
  }
  uiManager_->animationTick();
}

Why This Fix Works: It prevents the null pointer dereference by checking uiManager_ validity before use. During shutdown, if the scheduler is accessed after uiManager_ becomes invalid, the method will safely return instead of crashing.

Related Diffs

  • D92986523: Similar fix for null scheduler check in driveCxxAnimations()

Logview link: b3d4c4d8f7e6dd50b09fb7df9a1ad66a

CI

Differential Revision: D93363797

Pulled By: shubhamksavita

…ck for uiManager_ (#56128) (#56128)

Summary:

Changelog: [Android] [Fixed] - Fix crash in Scheduler::animationTick when uiManager_ is null.

Test Plan:
CI

Fixes a null pointer dereference crash in `Scheduler::animationTick()` that occurs during shutdown race conditions.

### Root Cause Analysis

**The Symptom**: `uiManager_` is null when `Scheduler::animationTick()` is called, causing a crash at offset 0x50 from null when accessing members of `UIManager`.

**The Root Cause**: During shutdown, when `uninstallFabricUIManager()` is called, the Choreographer's animation frame callback (`doFrame`) can still arrive. While `FabricUIManagerBinding::driveCxxAnimations()` checks for null scheduler (added in D92986523), the internal `Scheduler::animationTick()` method didn't check if `uiManager_` is valid before dereferencing it:

```cpp
void Scheduler::animationTick() const {
  uiManager_->animationTick();  // No null check - crashes if uiManager_ is null
}
```

**The Fix**: Added a null check for `uiManager_` before accessing it, following the same defensive pattern used in `driveCxxAnimations()` and other methods in the codebase:

```cpp
void Scheduler::animationTick() const {
  if (!uiManager_) {
    return;
  }
  uiManager_->animationTick();
}
```

**Why This Fix Works**: It prevents the null pointer dereference by checking `uiManager_` validity before use. During shutdown, if the scheduler is accessed after `uiManager_` becomes invalid, the method will safely return instead of crashing.

### Related Diffs
- D92986523: Similar fix for null scheduler check in `driveCxxAnimations()`

Logview link: [b3d4c4d8f7e6dd50b09fb7df9a1ad66a](https://www.internalfb.com/logview/system_vros_crashes/b3d4c4d8f7e6dd50b09fb7df9a1ad66a)

CI

Differential Revision: D93363797

Pulled By: shubhamksavita
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 24, 2026
@meta-codesync

meta-codesync Bot commented Jun 24, 2026

Copy link
Copy Markdown

@shubhamksavita has exported this pull request. If you are a Meta employee, you can view the originating Diff in D93363797.

@cortinico cortinico left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review automatically exported from Phabricator review in Meta.

@meta-codesync meta-codesync Bot closed this in e73592b Jun 25, 2026
@meta-codesync meta-codesync Bot added the Merged This PR has been merged. label Jun 25, 2026
@meta-codesync

meta-codesync Bot commented Jun 25, 2026

Copy link
Copy Markdown

@shubhamksavita merged this pull request in e73592b.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. meta-exported p: Facebook Partner: Facebook Partner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants