511440 - IPC protocol classes should use leak counting macros

Status: RESOLVED FIXED

(Core :: IPC, defect, P3)

Description

[Benjamin Smedberg]

In mozilla we typically use leak-counting macros to detect and prevent leaks: MOZ_COUNT_CTOR and MOZ_COUNT_DTOR. The IPDL-generated parent/child classes should use these macros in their constructor/destructors so that the automated leak-counting machinery can help us catch leaks.

See https://developer.mozilla.org/en/Debugging_memory_leaks#How_to_instrument_your_objects_for_refcnt.2fmemory_logging for some (sparse) documentation.

Comment 1

[Robin Bate Boerop]

Attachment: draft1

My first draft.

I used XPCOM_MEM_BLOAT_LOG to obtain a log of activity from a run of "firefox -chrome chrome://global/content/test-ipc.xul", to ensure that the constructors and destructors are being logged appropriately.  However, that does not exercise all of the objects generated by the IPDL compiler.  Does anyone have an idea of how to better exercise E10s?

Comment 2

[Benjamin Smedberg]

That will exercize the tab objects but not the plugin objects, but as long as the patch is suitably generic, that sounds ok to me. Are there particular objects you're worried about that we aren't counting?

Comment 3

[Robin Bate Boerop]

Yes. In the bloat log, I see evidence that an IFrameEmbeddingProtocolChild object was created, but not an IFrameEmbeddingProtocolParent. I would expect the latter. Also, I see no evidence that there has been any message object (like Msg_loadURL) created. It is as if there is no actual IPC taking place.

Comment 4

[Benjamin Smedberg]

This is one of many cases where debugging information (leak stats in this case) is going to be confused between the parent and child process: you are seeing the leakstats for the chrome process, which will have IFrameEmbeddingProtocolChild objects (but not protocolchild objects). When the content process shuts down you should see IFrameEmbeddingProtocolParent object stats.

As far as I know currently we don't shut down the content process cleanly. I expect this means that the code you have here is correct, it's just not capable of being used yet on the content process.

As for message objects, it sounds like we should leak-check those as well. I don't know how those are managed though; cjones/bent can provide more guidance.

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

The parent process will always create *ProtocolParent objects, and the child *ProtocolChild objects.  Robin, I don't know how to explain your observation without more information.

I don't think it's worth leak-checking Message objects.  These are automatically managed by IPDL; clients of IPDL never see them.

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to comment #5)
> The parent process will always create *ProtocolParent objects, and the child
> *ProtocolChild objects.
> 

Whoops, nope!  bsmedberg is right.

Comment 7

[Robin Bate Boerop]

Attachment: Patch, without MOZ_COUNT_CTOR/DTOR in Msg_ classes

Comment 8

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to comment #6)
> (In reply to comment #5)
> > The parent process will always create *ProtocolParent objects, and the child
> > *ProtocolChild objects.
> > 
> 
> Whoops, nope!  bsmedberg is right.

Whoops, nope!  bsmedberg is wrong.  I did away with this insanity a while back for this very reason, it was confusing.

So to be clear: the parent process will always create *ProtocolParent objects, and the child *ProtocolChild objects.

Comment 9

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Pushed http://hg.mozilla.org/projects/electrolysis/rev/c25f8028ecdf