Closed Bug 1331055 Opened 6 years ago Closed 6 years ago
Asynchronous creation of Android UI ns
Thread was potentially racey .
The asynchronous creation of the Android UI nsThread was potentially racey because the thread pointer could be potentially accessed before the thread was initialized.
Attachment #8826725 - Flags: review?(nchen)
Comment on attachment 8826725 [details] [diff] [review] 0001-Bug-1331055-Ensure-the-Android-UI-nsThread-has-been-initialized-before-making-it-accessible-17011311-8ac9bfb.patch Review of attachment 8826725 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/android/AndroidUiThread.cpp @@ +124,5 @@ > + RefPtr<AndroidUiThread> thread = new AndroidUiThread(); > + thread->InitCurrentThread(); > + thread->SetObserver(new ThreadObserver()); > + sMessageLoop = new MessageLoop(MessageLoop::TYPE_MOZILLA_ANDROID_UI, thread.get()); > + sThread = thread; So this can still be racy because to other cores/processors, `sThread = thread;` can appear to execute _before_ the previous lines. It's best to either 1) rid of double-checked locking in `GetAndroidUiThread()`, 2) add memory barriers here and in `GetAndroidUiThread()` around sThread accesses, or 3) use something that adds memory barriers for you, like Atomic<> from mfbt/Atomics.h. If you really don't want to take the lock in `GetAndroidUiThread()`, I think the next best choice is to wrap sThread in an Atomic<> object.
Attachment #8826725 - Flags: review?(nchen) → review-
Comment on attachment 8827612 [details] [diff] [review] 0001-Bug-1331055-Ensure-the-Android-UI-nsThread-has-been-initialized-before-making-it-accessible.-r-jchen-17011712-ee96df4.patch I hope this address the race condition in initialization.
Attachment #8827612 - Flags: review?(nchen)
Attachment #8827612 - Flags: review?(nchen) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/0a9fef37eeae Ensure the Android UI nsThread has been initialized before making it accessible. r=jchen
You need to log in before you can comment on or make changes to this bug.