Closed Bug 1343335 Opened 3 years ago Closed 3 years ago

Add asserts to check for null AbstractThread targets in MozPromise

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: qdot, Assigned: qdot)

Details

Attachments

(1 file)

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: nobody → kyle
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
https://hg.mozilla.org/mozilla-central/rev/f985ae4754e0
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.