Closed Bug 1110938 Opened 7 years ago Closed 7 years ago

Add a timeout for CPOWs


(Core :: IPC, defect)

Not set





(Reporter: billm, Assigned: billm)




(2 files)

We removed CPOW timeouts in bug 990598 because they didn't really work. Once the reply to the timed out message eventually came in, the main process would get confused and crash or behave strangely.

However, we really need a mechanism to time out CPOWs.
This is a preliminary patch that ensures that, if a transaction is initiated by message M, then the transaction ID is the same as the sequence number of M. This is a useful property for the later patch.
Attachment #8535750 - Flags: review?(dvander)
Attached patch timeout-cpowsSplinter Review
This patch adds CPOW timeouts. I kept thinking up corner cases where it wouldn't work, so there are a lot of restrictions about when it applies.

We only time out a message if it initiated a transaction. So if there's a lot of message nesting and an inner message is taking too long, then we won't time it out. I doubt this is really an issue, since nested messages means that the child should be blocked waiting for us.

When a message times out, we don't tell the other side to cancel it or anything. We still expect the other side to process the timed out message and send a reply.

Between when we time out a message and when we get a reply for it, any sync message sends in the parent will fail. This way we don't get a cascade of CPOWs and have to wait for each one to time out.

Additionally, if the parent has not yet received a reply for a timed out message, it will return an error for any incoming sync message. This is kind of unfortunate, but if we don't do it, we get problems where the parent and child both run sync messages simultaneously, which breaks all sorts of assumptions.
Attachment #8535768 - Flags: review?(dvander)
Attachment #8535750 - Flags: review?(dvander) → review+
Comment on attachment 8535768 [details] [diff] [review]

Review of attachment 8535768 [details] [diff] [review]:

Looks good to me, will be curious how it plays out versus the older timeout attempt.
Attachment #8535768 - Flags: review?(dvander) → review+

I defaulted the timeout to 0. I'm hoping to implement a better mechanism to detect when to timeout, and that will supersede the timer approach.
Also, I had to increase the b2g leak threshold since MessageChannel got a little bigger.
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in before you can comment on or make changes to this bug.