Fix crash on HeadlessJsTaskService on old architecture #48124
Conversation
facebook-github-bot
added
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D66826271 |
Summary: Fixes facebook#47592 The logic in HeadlessJsTaskService is broken. We should not check whether `getReactContext` is null or not. Instead we should use the `enableBridgelessArchitecture` feature flag to understand if New Architecture was enabled or not. The problem we were having is that `HeadlessJsTaskService` was attempting to load the New Architecture even if the user would have it turned off. The Service would then die attempting to load `libappmodules.so` which was correctly missing. Changelog: [Android] [Fixed] - Fix crash on HeadlessJsTaskService on old architecture Reviewed By: javache Differential Revision: D66826271
cortinico
force-pushed
the
export-D66826271
branch
from
e0c6242 to
3c747eb
Compare
|
This pull request was exported from Phabricator. Differential Revision: D66826271 |
There was a problem hiding this comment.
Fantastic! I integrated this locally and hit a build failure.
Suggested edit appears to fix build error and then the whole diff fixed the error I saw in my reproducer
Thanks @cortinico
| getReactNativeHost().getReactInstanceManager(); | ||
|
|
||
| reactInstanceManager.addReactInstanceEventListener( | ||
| if (ReactNativeFeatureFlags.enableBridgelessArchitecture()) { |
There was a problem hiding this comment.
Tested this locally, worked with this slight tweak:
| if (ReactNativeFeatureFlags.enableBridgelessArchitecture()) { | |
| if (ReactFeatureFlags.enableBridgelessArchitecture) { |
There was a problem hiding this comment.
Yeah that's because we replaced ReactFeatureFlags with ReactNativeFeatureFlags on main, but on the release branch we still reference the old class
There was a problem hiding this comment.
Ah of course, and my reproducer was 0.76.2 still (not even .3)
I was confused how that even compiled otherwise and it was sailing right through CI! Makes sense
Okay, then given this was really just symbol motion, not really a semantic change, you've nailed it.
|
Interesting. I think I managed to step around this bug in my own version where I borrowed the guts from So it was just getting the wires crossed between use of |
|
This pull request has been merged in 4560fc0. |
Summary: Pull Request resolved: #48124 Fixes #47592 The logic in HeadlessJsTaskService is broken. We should not check whether `getReactContext` is null or not. Instead we should use the `enableBridgelessArchitecture` feature flag to understand if New Architecture was enabled or not. The problem we were having is that `HeadlessJsTaskService` was attempting to load the New Architecture even if the user would have it turned off. The Service would then die attempting to load `libappmodules.so` which was correctly missing. Changelog: [Android] [Fixed] - Fix crash on HeadlessJsTaskService on old architecture Reviewed By: javache Differential Revision: D66826271 fbshipit-source-id: 2b8418e0b01b65014cdbfd0ec2f843420a15f9db
Summary: Pull Request resolved: #48124 Fixes #47592 The logic in HeadlessJsTaskService is broken. We should not check whether `getReactContext` is null or not. Instead we should use the `enableBridgelessArchitecture` feature flag to understand if New Architecture was enabled or not. The problem we were having is that `HeadlessJsTaskService` was attempting to load the New Architecture even if the user would have it turned off. The Service would then die attempting to load `libappmodules.so` which was correctly missing. Changelog: [Android] [Fixed] - Fix crash on HeadlessJsTaskService on old architecture Reviewed By: javache Differential Revision: D66826271 fbshipit-source-id: 2b8418e0b01b65014cdbfd0ec2f843420a15f9db
Summary:
Fixes #47592
The logic in HeadlessJsTaskService is broken. We should not check whether
getReactContextis null or not.Instead we should use the
enableBridgelessArchitecturefeature flag to understand if New Architecture was enabled or not.The problem we were having is that
HeadlessJsTaskServicewas attempting to load the New Architecture even if the user would have it turned off. The Service would then die attempting to loadlibappmodules.sowhich was correctly missing.Changelog:
[Android] [Fixed] - Fix crash on HeadlessJsTaskService on old architecture
Differential Revision: D66826271