Repository navigation
Commit 65b3e64
Fix race in UiThreadUtil main handler lazy initialization (#58963)
Summary:
`UiThreadUtil` keeps its main-thread `Handler` in a property delegated to `lazy(LazyThreadSafetyMode.NONE)`:
```kotlin
private val mainHandler: Handler by
lazy(LazyThreadSafetyMode.NONE) { Handler(Looper.getMainLooper()) }
```
`NONE` creates an `UnsafeLazyImpl`, whose getter is:
```kotlin
if (_value === UNINITIALIZED_VALUE) {
_value = initializer!!()
initializer = null
}
return _value as T
```
`mainHandler` is first read by whichever thread first calls `runOnUiThread()`, `getUiThreadHandler()` or `removeOnUiThread()`. `isOnUiThread()` and the `assert*` helpers only use `Looper`, so the main thread calling them does not initialize it. In practice the first reads usually come from background threads posting to the UI thread, such as the JS thread, the native modules thread, or a library's executor. When two of them make their first call at the same time:
1. Thread A sees `_value === UNINITIALIZED_VALUE` and starts the initializer.
2. Thread B also sees `UNINITIALIZED_VALUE`, because A has not written `_value` yet.
3. A writes `_value` and sets `initializer = null`.
4. B evaluates `initializer!!()` and throws `NullPointerException`.
The same NPE can happen without the two calls overlapping in time. `_value` and `initializer` are plain fields, so on a weakly ordered CPU (ARM) another thread can observe `initializer == null` before it observes the new `_value`.
The exception is thrown from the accessor Kotlin generates for the delegated property, so crash reports show the top frame as `com.facebook.react.bridge.UiThreadUtil.getMainHandler`. Only the first access in a process can race, so the crash is rare but fatal and usually happens during startup.
This was introduced in #50536 (1033584), which migrated `UiThreadUtil` to Kotlin. The Java implementation used double-checked locking on a `volatile` field. A reviewer [suggested](#50536 (comment)) plain `by lazy { }`, which is synchronized by default, but the merged code uses `LazyThreadSafetyMode.NONE`. Every release from 0.80 to 0.88.0-rc.4 includes it.
This PR switches the property to `LazyThreadSafetyMode.SYNCHRONIZED`, restoring the guarantee of the Java implementation. The lock is only taken until the value is initialized. After that, reads are a single volatile read, so the `runOnUiThread` hot path is effectively unchanged.
## Changelog:
[ANDROID] [FIXED] - Fix crash in `UiThreadUtil` when the main handler is first accessed from multiple threads at once
Pull Request resolved: #58963
Test Plan:
The race only happens on the first access in a process and cannot be reproduced reliably in a unit test. It can be confirmed by:
1. Kotlin stdlib: `UnsafeLazyImpl` (`NONE`) is documented as unsafe for multi-threaded access. `SynchronizedLazyImpl` (`SYNCHRONIZED`) guarantees a single initialization and safe publication.
2. Bytecode: in a release APK built with React Native 0.86.0, `UiThreadUtil.<clinit>` creates the lazy with `LazyThreadSafetyMode.NONE`. With this change it uses `SYNCHRONIZED`.
3. Behavior: the public API and the value returned are unchanged. The only difference is how the handler is initialized the first time.
Reviewed By: javache
Differential Revision: D126040472
Pulled By: cortinico
fbshipit-source-id: 49753fac5210be481cc1fb843f2eb4594605cb6a1 parent 7fc58f2 commit 65b3e64
1 file changed
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
18 | | - | |
| 18 | + | |
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
| |||
0 commit comments