Skip to content

fix(Android, Stack v4): Clear supportActionBar on fragment removal to prevent ScreenStackFragment leak#3867

Merged
t0maboro merged 4 commits intomainfrom
@t0maboro/header-leaks
Apr 13, 2026
Merged

fix(Android, Stack v4): Clear supportActionBar on fragment removal to prevent ScreenStackFragment leak#3867
t0maboro merged 4 commits intomainfrom
@t0maboro/header-leaks

Conversation

@t0maboro
Copy link
Copy Markdown
Contributor

@t0maboro t0maboro commented Apr 10, 2026

Description

Detected retention chain:

  • AppCompatDelegateImpl.mActionBar
  • ToolbarActionBar.mDecorToolbar
  • ToolbarWidgetWrapper.mToolbar
  • DebugMenuToolbar.config
  • ScreenStackHeaderConfig.mParent
  • Screen.fragment

ScreenStackHeaderConfig.onUpdate calls activity.setSupportActionBar(toolbar) on top screen updates. AppCompatDelegateImpl stores the resulting ToolbarActionBar in mActionBar for the lifetime of the activity. When a screen is popped (and the new top screen does not install a replacement ActionBar) the stale ToolbarActionBar is never released.

In onDestroyView of ScreenStackFragment, call reset SupportActionBar to release it and the fragment. I'm storing a reference to actionBar that ensures the call is skipped when the incoming screen has already installed its own toolbar in onUpdate.

Closes: https://github.com/software-mansion/react-native-screens-labs/issues/1129

Changes

  • added a reference to the actionBar in ScreenStackHeaderConfig to clear actionBar only if it's associated with that fragment and wasn't overridden by other screen
  • added onDestroyView override in ScreenStackFragment that will clear the ActionBar
  • changed setToolbar signature to pass a CustomToolbar, which allows for storing the header config - HeaderConfig is nulled by React transaction before FragmentManager sends onDestroy to ScreenStackFragment

Before & after - visual documentation

Before After
before.mov
after.mov

Test plan

Test3867

Checklist

  • Included code example that can be used to test this change.
  • For visual changes, included screenshots / GIFs / recordings documenting the change.
  • For API changes, updated relevant public types.
  • Ensured that CI passes

Copy link
Copy Markdown
Contributor

@kligarski kligarski 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 might've found another leak:

Screen.Recording.2026-04-13.at.12.58.31.mov

But I guess that it might be unrelated to this PR.

@t0maboro
Copy link
Copy Markdown
Contributor Author

Looks good.

I might've found another leak:

Screen.Recording.2026-04-13.at.12.58.31.mov
But I guess that it might be unrelated to this PR.

This one might be the result that we have some nested stacks, which screens aren't displayed at this time, but I'll check #3855 because that one might help here. I had a similar case when using JS tabs https://github.com/software-mansion/react-native-screens-labs/issues/1130 but in that one there are 3 paths and the main problem is that these Screen instances are attached in the view hierarchy all the time.

To sum up, if that PR with freeing fragmentWrapper is sufficient, I think we may try to proceed in that one, but if in your case, all resources will be deallocated when you'll go back to the 1st screen - this isn't memory leak, rather an issue in our Model (especially in detachScreen method) that we're trying to remove fragments instead of detaching them.

@t0maboro t0maboro force-pushed the @t0maboro/header-leaks branch from 9d02d76 to 07f58f5 Compare April 13, 2026 12:05
@t0maboro t0maboro force-pushed the @t0maboro/header-leaks branch from 07f58f5 to b02f241 Compare April 13, 2026 12:35
@t0maboro t0maboro merged commit 01375a5 into main Apr 13, 2026
5 checks passed
@t0maboro t0maboro deleted the @t0maboro/header-leaks branch April 13, 2026 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants