Closed
Bug 1343335
Opened 7 years ago
Closed 7 years ago
Add asserts to check for null AbstractThread targets in MozPromise
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: qdot, Assigned: qdot)
Details
Attachments
(1 file)
2.90 KB,
patch
|
bholley
:
review+
|
Details | Diff | Splinter Review |
AbstractThread::GetCurrent() can return null if a thread isn't a default AbstractThread (MainThread, etc). This doesn't get caught in MozPromise until we try to check for reliability of dispatching on the thread, a check that can probably be removed now as it always returns true. However, this makes the crash reason opaque enough that it needs to be opened up in the debugger. Adding asserts to save others some time I already lost.
Assignee | ||
Comment 1•7 years ago
|
||
Attachment #8842149 -
Flags: review?(bobbyholley)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → kyle
Comment 2•7 years ago
|
||
Comment on attachment 8842149 [details] [diff] [review] Patch 1 (v1) - Add asserts to check for null AbstractThread in MozPromise Review of attachment 8842149 [details] [diff] [review]: ----------------------------------------------------------------- r=me with that. ::: xpcom/threads/MozPromise.h @@ +352,5 @@ > const char* aCallSite) > : mResponseTarget(aResponseTarget) > , mCallSite(aCallSite) > + { > + MOZ_ASSERT(aResponseTarget, "AbstractThread target should not be null!"); Nit: I don't think these messages are useful - please use the one-argument version of MOZ_ASSERT.
Attachment #8842149 -
Flags: review?(bobbyholley) → review+
Pushed by kmachulis@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f985ae4754e0 Add asserts to check for null AbstractThread targets in MozPromise; r=bholley
Comment 4•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f985ae4754e0
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•