854880 - Canceling a RunnableFunction does not do anything

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Justin Lebar (not reading bugmail)]

ipc::RunnableFunction::Cancel() is empty.  That is...not what I expected that function to do.

Patch in a moment.

Comment 1

[Justin Lebar (not reading bugmail)]

Attachment: Patch, v1

This is such a wtf, I find it hard to believe it's unintentional.  But I can't
see why one would want this behavior, and it's breaking code we have in
ContentChild that naively expects canelling a runnable will prevent it from
running.

Am I missing something, Ben?

Comment 2

[Justin Lebar (not reading bugmail)]

https://tbpl.mozilla.org/?tree=Try&rev=2a27e9aada69

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 729506 [details] [diff] [review]
Patch, v1

Review of attachment 729506 [details] [diff] [review]:
-----------------------------------------------------------------

r=me.

Comment 4

[Justin Lebar (not reading bugmail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f1b0e51c1b92

Comment 5

[Justin Lebar (not reading bugmail)]

tef? -- This indirectly blocks a blocker.

This is actually a bit scary, since it's hard to find all the people who call Cancel() and make sure that they don't blow up if the task is not canceled.  We might be able to work around this bug on branches the blocking issue by making only this one task be canceled, but there are many costs to fragmenting our code base like that.  In particular, since m-c gets essentially zero b2g testing, checking in a branch-only change would essentially remove any b2g testing of this patch on m-c.  Then when we eventually switch back to m-c, we'll have big problems.

I would prefer to avoid that.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f1b0e51c1b92

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/bacaaa4ae31f
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/6bd8fab5b597