Remove the need to call Mutex::ShutDown()
Categories
(Core :: JavaScript Engine, task, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox71 | --- | fixed |
People
(Reporter: pbone, Assigned: pbone)
References
Details
Attachments
(10 files, 3 obsolete files)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review |
Two fuzzbugs Bug 1567292 and Bug 1562437 have a similar cause. After using a JS_Context a caller should call Mutex::ShutDown() to free some debugging information there. Maybe we should change JS_NewContext somehow to force the caller to free this. Or do it in JS_DestroyContext() if we're the last context for this thread.
Comment 1•5 years ago
|
||
(In reply to Paul Bone [:pbone] from comment #0)
Or do it in JS_DestroyContext() if we're the last context for this thread.
There can be at most one context per thread, so in JS_DestroyContext it's always the thread's last context :)
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 2•5 years ago
|
||
If we call Mutex::Shutdown from here then most embedders' threads that
interact with JS will do the right thing.
Assignee | ||
Comment 3•5 years ago
|
||
If we call Mutex::ShutDown() in these destructors then we can remove it from
several threads.
Assignee | ||
Comment 4•5 years ago
|
||
Factor the common parts of the DEBUG and non-DEBUG Mutex class together to
make it easier to see what the common and different parts of this class are.
Assignee | ||
Comment 5•5 years ago
|
||
Implement this as a linked list so that it cannot leak memory (unless it
also leaks locks) and the Mutex::ShutDown() function can be removed.
Depends on D40984
Assignee | ||
Comment 6•5 years ago
|
||
Depends on D40985
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 7•5 years ago
|
||
Condition variables lock and unlock mutexes. The code in the next patch
would find assertion failures because this case wasn't handled.
Depends on D40984
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 8•5 years ago
|
||
Part 6 will #include Thread.h from Mutex.h, we can change the mutex used by
Thread.h so that Thread.h doesn't need to include Mutex.h.
Depends on D40986
Assignee | ||
Comment 9•5 years ago
|
||
While I worked on this patch series these assertions where helpful to detect
problems.
Depends on D43003
Assignee | ||
Comment 10•5 years ago
|
||
We can add an extra assertion here.
Depends on D43004
Assignee | ||
Comment 11•5 years ago
|
||
Depends on D43005
Updated•5 years ago
|
Assignee | ||
Comment 12•5 years ago
|
||
Depends on D40986
Assignee | ||
Comment 13•5 years ago
|
||
Depends on D44430
Assignee | ||
Comment 14•5 years ago
|
||
Depends on D44431
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Comment 15•5 years ago
|
||
Pushed by pbone@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/01952ffb255b (part 1) Refactor Mutex.h r=jandem https://hg.mozilla.org/integration/autoland/rev/371db20a6eec (part 2) Call the pre/post lock code from ConditionVariable r=jandem https://hg.mozilla.org/integration/autoland/rev/8e7f65fa0100 (part 3) Implement the mutex stack as a linked list r=jandem https://hg.mozilla.org/integration/autoland/rev/27fd4f64ae73 (part 4) Remove Mutex::ShutDown() r=jandem https://hg.mozilla.org/integration/autoland/rev/d7244290aa1f (part 5) Move Thread::Id to ThreadId and its own header r=jandem https://hg.mozilla.org/integration/autoland/rev/77edb2b4c0c3 (part 6) Move ThisThread::GetId() to ThreadId::ThisThread() r=jandem https://hg.mozilla.org/integration/autoland/rev/5cd99fb56ad6 (part 7) Remove references to Thread::Id r=jandem https://hg.mozilla.org/integration/autoland/rev/eca5bab9590e (part 8) Add assertions to help catch problems Mutex behaviour r=jandem https://hg.mozilla.org/integration/autoland/rev/4006bbe265cb (part 9) Verify ownedByCurrentThread() using owningThread_ r=jandem https://hg.mozilla.org/integration/autoland/rev/5a173762d443 (part 10) Clarify an edge case in a comment r=jandem
Comment 16•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/01952ffb255b
https://hg.mozilla.org/mozilla-central/rev/371db20a6eec
https://hg.mozilla.org/mozilla-central/rev/8e7f65fa0100
https://hg.mozilla.org/mozilla-central/rev/27fd4f64ae73
https://hg.mozilla.org/mozilla-central/rev/d7244290aa1f
https://hg.mozilla.org/mozilla-central/rev/77edb2b4c0c3
https://hg.mozilla.org/mozilla-central/rev/5cd99fb56ad6
https://hg.mozilla.org/mozilla-central/rev/eca5bab9590e
https://hg.mozilla.org/mozilla-central/rev/4006bbe265cb
https://hg.mozilla.org/mozilla-central/rev/5a173762d443
Description
•