Closed Bug 1313989 Opened 3 years ago Closed 3 years ago

Get rid of the MutexAutoUnlock in nsThread::GetIdleEvent

Categories

(Core :: DOM: Core & HTML, defect, P3)

48 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: farre, Assigned: farre)

References

Details

Attachments

(1 file)

It would be better if we didn't release the lock while expecting events.

This also means that nsIIdlePeriod::GetIdlePeriodHint never should post runnables to the current thread.
Depends on: 1198381
Priority: -- → P3
Assignee: nobody → afarre
Comment on attachment 8809317 [details]
Bug 1313989 - Remove MutexAutoUnlock in nsThread::GetIdleEvent.

https://reviewboard.mozilla.org/r/91906/#review91948

r=me on the XPCOM parts.  Is there some assert we can stick somewhere so doing IPC in situations like this would result in a crash rather than a deadlock?

::: layout/ipc/VsyncChild.h:41
(Diff revision 1)
>    TimeDuration GetVsyncRate();
> +  TimeDuration VsyncRate();

I don't know what Matt thinks, but I think there ought to be some comment explaining why we have two different functions that appear by their names to do the same thing.  The comment in nsRefreshDriver is helpful, but some of it surely belongs here as well.
Attachment #8809317 - Flags: review?(nfroyd) → review+
Comment on attachment 8809317 [details]
Bug 1313989 - Remove MutexAutoUnlock in nsThread::GetIdleEvent.

https://reviewboard.mozilla.org/r/91906/#review91948

I guess that something like the asserts in the beginning of: https://dxr.mozilla.org/mozilla-central/source/nsprpub/pr/src/pthreads/ptsynch.c#198-201 but inverted.

That is, if the current thread owns the lock and we're trying to grab it, then assert?

> I don't know what Matt thinks, but I think there ought to be some comment explaining why we have two different functions that appear by their names to do the same thing.  The comment in nsRefreshDriver is helpful, but some of it surely belongs here as well.

Definitely. Will do.
Comment on attachment 8809317 [details]
Bug 1313989 - Remove MutexAutoUnlock in nsThread::GetIdleEvent.

https://reviewboard.mozilla.org/r/91906/#review92108
Attachment #8809317 - Flags: review?(matt.woodrow) → review+
Pushed by afarre@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/188b07c3d90a
Remove MutexAutoUnlock in nsThread::GetIdleEvent. r=froydnj,mattwoodrow
https://hg.mozilla.org/mozilla-central/rev/188b07c3d90a
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.