IPDL Best Practices

There a range of mistakes that authors of new sub-protocols frequently make.  This document is intended to help correct these before the formal review process is initiated.

Lifetime Management

Ensure IPDL deletes your protocol

All protocols must be deleted.  Period.  Are you sending the __delete__ message to trigger protocol deletion?  This one is easy to miss, as the corresponding Recv__delete__ function has a stub implementation automatically generated in the base class.  Simply calling delete on a pointer to an actor is not enough.

Using IPDL generated structs as data structures outside of IPC serialization/deserialization

IPDL generated structures are meant to facilitate serialization/deserialization and message passing. Do not use them as data structures outside of IPDL, or wou will eventually find yourself in bad situations to implement proper memory management. A good example of this problem is the amount of memory management bugs that we have had with using SurfaceDescriptor directly everywhere.

Implement (preferably reference counted) classes to wrap the shared data instead of letting several objects reference SurfaceDescriptors or their content directly.

Reference counting protocol actors is tricky

Here is the easiest way to get it right the first time (lessons learned from the http channel and geolocation protocols):

• AllocPProtocol calls AddRef
• DeallocPProtocol calls Release

This ensures that the actor will not disappear from under IPDL, and that you won't get bizarre crashes at other times if IPDL deletes the protocol tree.  The main concern you have now is that you absolutely, certainly, positively call Send__delete__ in all possible circumstances or your protocol can and will leak.

Calling IPDL functions willy-nilly

Let's say you have an object that implements an IPDL interface, but it also outlives it (case in point: reference-counted).  Is there any chance that an IPDL function could be called after the protocol itself has been destroyed, or before it has been created?  Don't let it happen - that is a guaranteed crash right there.  Use a flag and the generated ActorDestroy function to control when IPDL functions can be called; see PHttpChannelParent/Child for an example.

When to run code

Here's a rundown on appropriate places to run certain kinds of code:

• Manager::AllocPProtocol - allocation
• Manager::RecvPProtocolConstructor - initialization, protocol deletion (the TypeAheadFind protocol uses one-shot protocols like this)
• Actor::Recv__delete__ - cleanup, IPDL calls still valid but ill-advised
• Actor::ActorDestroy - non-IPDL cleanup
• Manager::DeallocPProtocol - deallocation

One construct to avoid:

class ProtocolParent {
public:
    ~ProtocolParent() {
        Send__delete__(this);  
    }
};
class ProtocolManagerParent {
public:
    PProtocolParent* AllocPProtocol() {
        return new ProtocolParent();
    }
    bool DeallocPProtocol() {
        return true;
    }
};
nsAutoPtr<PProtocolParent> parent = manager->SendPProtocolConstructor();

While this may look appealing as the IPDL logic is stuffed away in a sensible place, it breaks under error conditions.  Specifically, because AllocPProtocol and DeallocPProtocol are asymmetric, if IPDL ends up shutting down the protocol tree due to an error, the auto pointer will attempt to trigger protocol deletion a second time.  While this can be worked around with ActorDestroy, being explicit about deleting the protocol with Send__delete__ is easier to maintain, with symmetric Alloc/DeallocPProtocol functions also being easier to reason about.

Minimize overall use of messages

Reducing IPC traffic is a righteous goal.  Consider the following protocol:

async protocol PAsyncQuerier {
child:
  PAsyncQuery();
} 
async protocol PAsyncQuery {
child:
  KickOffQuery(nsString query);
parent:
  ReturnResult(nsString result);
  __delete__();
} 

In this situation, there is a guaranteed sequence of messages that will be sent.  Following actor construction, the parent will send a KickOffQuery message and presumably ignore the actor until it receives ReturnResult, at which point it will be deleted.  It makes sense to fold construction and the first message together, as well as the penultimate and deletion messages, so that the final protocol looks like this:

async protocol PAsyncQuerier {
child:
  PAsyncQuery(nsString query);
} 
async protocol PAsyncQuery {
parent:
  __delete__(nsString result);
}