Add asserts to check for null AbstractThread targets in MozPromise

RESOLVED FIXED in Firefox 54

Status

()

Core
XPCOM
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: qdot, Assigned: qdot)

Tracking

unspecified
mozilla54
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

Attachments

(1 attachment)

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.
Created attachment 8842149 [details] [diff] [review]
Patch 1 (v1) - Add asserts to check for null AbstractThread in MozPromise
Attachment #8842149 - Flags: review?(bobbyholley)
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+

Comment 3

a year ago
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

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f985ae4754e0
Status: NEW → RESOLVED
Last Resolved: a year 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.