Closed Bug 1350688 Opened 3 years ago Closed 3 years ago

SpinEvent ctor implicitly depends on telemetry eagerly loading osfile.jsm on Windows


(Core :: IPC, defect)

Not set



Tracking Status
firefox55 --- fixed


(Reporter: mccr8, Assigned: mccr8)




(1 file)

SpinEvent::SpinEvent() appears to be called off the main thread, and to figure out if there is more than one CPU it calls do_GetService(NS_SYSTEMINFO_CONTRACTID), which is implemented by nsSystemInfo. nsSystemInfo::Init can't be called off the main thread (at least on Windows, in whatever conditions cause GetProfileHDDInfo() to return false) because it tries to add an observer, and the observer service can't be called off the main thread. So, this code implicitly depends on nsSystemInfo having been initialized earlier.

Right now, telemetry eagerly loads osfile.jsm early in startup on the main thread, which calls InitOSFileConstants() which calls do_GetService(";1") which calls nsSystemInfo::Init(), so it works out.

However, my patch in bug 1349389 makes telemetry load osfile.jsm lazily, so I get Windows crashes in a11y and e10s bc7:

Some possible fixes:
- Find some other way for SpinEvent to figure out if there is more than one CPU.
- Force the nsSystemInfo to always start up early on the main thread somehow.
- Dispatch a runnable to the main thread to deal with the observer service.

As a side note, as far as I can tell, the observer service setup in nsSystemInfo is not quite threadsafe, despite using threadsafe refcounting, because the hashtable for the property bag is not guarded by locks. It is possible for the main thread to get the profile-do-change topic and mutate the hashtable while another thread is reading from the hashtable. (Otherwise, the hashtable is only written during nsSystemInfo::Init so it should be okay.)
It looks like in bug 1311834, you were originally going to do some SYSTEM_INFO thing. Can I just use that here? There's also some PR_GetNumberOfProcessors() method I could use.
Flags: needinfo?(aklotz)
Assignee: nobody → continuation
Comment on attachment 8851365 [details]
Bug 1350688 - Use a thread safe way to get the CPU count in the SpinEvent ctor.

lgtm, thanks!
Attachment #8851365 - Flags: review?(aklotz) → review+
Pushed by
Use a thread safe way to get the CPU count in the SpinEvent ctor. r=aklotz
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.