Fix TIMOB-19903 Android: Fix crash due to GC of UI native proxies in hyperloop #7448
Conversation
…hyperloop - Need to fire on* property callbacks synchronously when run on main thread. Otherwise they fire _after_ we've already released the activity proxy on the native side and we get a fatal crash
| } else { | ||
| // This hopefully finishes before we release the reference on the native side?! I have seen it crash because it didn't before though... | ||
| // Not sure it's safe to keep this behavior... | ||
| activityProxy.callPropertyAsync(name, new Object[] { data }); |
There was a problem hiding this comment.
under what circumstances would this method be called on KrollRuntime thread? This is used exclusively on on* methods, which run on the main thread.
There was a problem hiding this comment.
I'm not sure I follow. With recent changes (required by hyperloop) the KrollRuntime thread may be the same as the UI thread or separate depending on a tiapp property.
callPropertyAsync always sticks a message in the queue to fire the JS method. When the KrollRuntime thread is the main/UI thread, that means it's guaranteed not to fire until after all the work it's currently doing - so it won't ever run until after we've already destroyed the activity proxy during onDestroy flows.
When there are separate main and UI threads, I'm not sure that there's any guarantee of ordering between the onDestroy/onStop callback and the release of the activity proxy. So I think it will effectively be a race condition. I have anecdotally seen this when debugging and not running on the main thread - where closing a window would cause a crash. But perhaps I'm wrong and we can guarantee/know that the on* callback will always be fired and finished before the release call later in the onDestroy flow?
There was a problem hiding this comment.
Ah, I'm not up to date with the recent hyperloop changes. If the callbacks are async then there's no guarantee that it will be finished before release call. This change should be fine.
Fix TIMOB-19903 Android: Fix crash due to GC of UI native proxies in hyperloop
This is actually related to running on main thread, not specifically hyperloop. Also, while this does fix the issue, it raises the question on whether we want to always call the on* callbacks sync, even if we're not running on main thread. It's entirely possible that the crash could happen if the timing is right when fired async for onStop/onDestroy.