Closed
Bug 1388464
Opened 8 years ago
Closed 8 years ago
Blocking preference write causes us to re-enter event loop
Categories
(Core :: Preferences: Backend, defect)
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox56 | --- | unaffected |
firefox57 | --- | fixed |
People
(Reporter: milan, Assigned: milan)
References
Details
Attachments
(1 file)
See 1386263. The goal of the sync pref file save off the main thread was to block the main thread until the work is done, not to allow an event loop to spin which is allowed to process other events.
Assignee | ||
Updated•8 years ago
|
Assignee | ||
Comment 1•8 years ago
|
||
We are using nsIEventTarget::Dispatch with DISPATCH_SYNC flag. We should have been using SyncRunnable::DispatchToThread instead.
Assignee | ||
Comment 2•8 years ago
|
||
Comment hidden (mozreview-request) |
Comment 4•8 years ago
|
||
mozreview-review |
Comment on attachment 8895073 [details]
Bug 1388464: Use SyncRunnable instead of DISPATCH_SYNC to actually block the main thread during the blocking preference file write.
https://reviewboard.mozilla.org/r/166198/#review171466
Attachment #8895073 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 5•8 years ago
|
||
Assignee | ||
Comment 6•8 years ago
|
||
NI Benjamin just to make sure he's seen this.
Flags: needinfo?(benjamin)
Pushed by msreckovic@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6ffe8a4a92c7
Use SyncRunnable instead of DISPATCH_SYNC to actually block the main thread during the blocking preference file write. r=smaug
Comment 8•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Updated•8 years ago
|
Flags: needinfo?(benjamin)
Comment 9•8 years ago
|
||
Didn't this code land for 56? I think this needs an uplift.
Assignee | ||
Comment 10•8 years ago
|
||
Indeed, but 56 has it disabled through a preference, so we shouldn't hit this codepath. But, yes, if we want to enable the off main thread preference write and on pref change auto save, we'd need this patch as well.
Flags: needinfo?(milan)
Assignee | ||
Comment 12•8 years ago
|
||
This is still disabled by pref in 56, so 56 is unaffected.
tracking-firefox56:
+ → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•