Closed Bug 540886 Opened 10 years ago Closed 10 years ago

IPDL: Implement a mechanism to force child actors to wait for IPC traffic

Categories

(Core :: IPC, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: mozilla+ben, Assigned: cjones)

References

Details

(Whiteboard: [land m-c])

Attachments

(3 files, 3 obsolete files)

The parent sends an async message called Attention(), that, when received by the child, causes the child to spin its IPC event loop and then wait for new messages, as if expecting a synchronous call to return.

The parent may then send messages to the child with the expectation that the child is doing nothing except handling the parent's messages.  In particular, the child cannot send synchronous messages to the parent, and any asynchronous messages it sends are deferred until later.

After handling each message, the child continues to wait, unless it receives the AtEase() message from its parent, whereupon it resumes normal activity (sending child->parent messages, servicing other event loops, &c.).

It should be an error for the parent to return to its own event loop without sending the AtEase() message.

I'm not committed to any particular terminology, but Attention()/AtEase() seems a bit more accurate than Freeze()/Unfreeze().  If it's possible to support the desired behavior without adding special methods to IPDL protocols, that would be even better.
(In reply to comment #0)
> If it's possible to support the
> desired behavior without adding special methods to IPDL protocols, that would
> be even better.

Agreed.  I'm going to add these to all top-level RPC actor parents.

Currently I like BlockChild()/UnblockChild(), but also don't care much about the terminology.
bent, again r?ing for potential backporting of this code.
Attachment #422850 - Flags: review?(bent.mozilla)
Not the greatest test ever, but this feature is pretty hard to test thoroughly.

benjamn, please let me if this interface serves your needs.  BTW, I left checking for UnblockChild-before-returning-to-event-loop for followup work.  I realized that I don't know how to check that ;).
Attachment #422853 - Flags: review?(bnewman)
Comment on attachment 422853 [details] [diff] [review]
Test for BlockChild()/UnblockChild()

Suits my needs, indeed.
Attachment #422853 - Flags: review?(bnewman) → review+
Attachment #422850 - Flags: review?(bent.mozilla) → review+
(In reply to comment #4)
> BTW, I left
> checking for UnblockChild-before-returning-to-event-loop for followup work.  I
> realized that I don't know how to check that ;).

In nsThread.cpp, there's an |AfterProcessNextEvent()| observer notification that might do the trick.
Comment on attachment 422906 [details] [diff] [review]
 Offer a BlockChild() interface to RPC protocols that allows parents to prevent children from sending messages back of their own volition until the parent calls UnblockChild(), v1.1

>+RPCChannel::OnSpecialMessage(uint16 id, const Message& msg)
> ...
>+    default:
>+        return AsyncChannel::OnSpecialMessage(id, msg);

Shouldn't we pass through to SyncChannel?

>+    if (!mChild)
>+        NS_RUNTIMEABORT("child tried to block parent");

Just noticed that we never initialize mChild in the AsyncChannel constructor (it's set on Open, it looks like), should probably do that :)

>+    while (1) {
> ...
>+        while (Connected() && mPending.empty() && mBlockedOnParent) {

I think you can make things much clearer with only a single while loop.

  while (mBlockedOnParent) {
    WaitForNotify();
    if (!Connected()) { ... }
    if (!mPending.empty()) { }
    }
  }
Attachment #422906 - Flags: review?(bent.mozilla) → review+
(In reply to comment #8)
> (From update of attachment 422906 [details] [diff] [review])
> >+RPCChannel::OnSpecialMessage(uint16 id, const Message& msg)
> > ...
> >+    default:
> >+        return AsyncChannel::OnSpecialMessage(id, msg);
> 
> Shouldn't we pass through to SyncChannel?
> 

SyncChannel doesn't have any special messages it cares about (yet).  Could add a placeholder I guess.
This interface would be great if it worked.  Every CPOW operation could request run-to-completion from the ContentProcess (only the first request would block the child), and the child would be automatically unblocked when the parent process returned to its event loop.

The problem: calling alert() (for example) in the parent process spins the parent event loop, unblocking the child before the parent has completed the current sequence of CPOW operations.

Is there any way to avoid calling UnblockChild when AfterProcessNextEvent results from an alert()?
We could lift the requirement that UnblockChild() be called before returning to the event loop.
Blocks: 542341
Comment on attachment 423476 [details] [diff] [review]
Manually counting event nesting depth with respect to RequestRunToCompletion.

See followup bug 542341.
Attachment #423476 - Attachment is obsolete: true
(In reply to comment #8)
> (From update of attachment 422906 [details] [diff] [review])
> >+    if (!mChild)
> >+        NS_RUNTIMEABORT("child tried to block parent");
> 
> Just noticed that we never initialize mChild in the AsyncChannel constructor
> (it's set on Open, it looks like), should probably do that :)
> 
> >+    while (1) {
> > ...
> >+        while (Connected() && mPending.empty() && mBlockedOnParent) {
> 
> I think you can make things much clearer with only a single while loop.
> 
>   while (mBlockedOnParent) {
>     WaitForNotify();
>     if (!Connected()) { ... }
>     if (!mPending.empty()) { }
>     }
>   }

(In reply to comment #8)
> (From update of attachment 422906 [details] [diff] [review])
> I think you can make things much clearer with only a single while loop.
> 
>   while (mBlockedOnParent) {
>     WaitForNotify();
>     if (!Connected()) { ... }
>     if (!mPending.empty()) { }
>     }
>   }

Shite!  I forgot to change these before pushing.  Will push with next cset.
You need to log in before you can comment on or make changes to this bug.