Last Comment Bug 717027 - IPDL compiler doesn't generate code to handle message type SHMEM_CREATED_MESSAGE_TYPE when no other async messages are declared
: IPDL compiler doesn't generate code to handle message type SHMEM_CREATED_MESS...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: IPC (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla12
Assigned To: Ali Juma [:ajuma]
:
: Bill McCloskey (:billm)
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-10 13:58 PST by Ali Juma [:ajuma]
Modified: 2012-01-14 01:34 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Make IPDL compiler generate code to handle SHMEM-related messages even when there are no other async messages (2.36 KB, patch)
2012-01-12 11:41 PST, Ali Juma [:ajuma]
cjones.bugs: review+
Details | Diff | Splinter Review

Description Ali Juma [:ajuma] 2012-01-10 13:58:43 PST
When the declaration of a manager protocol doesn't include any async messages received by the parent, the ipdl compiler does not generate code to handle messages of type SHMEM_CREATED_MESSAGE_TYPE in Parent::OnMessageReceived(Message&). A workaround is simply adding an (unused) async message to the protocol declaration.
Comment 1 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-01-10 22:55:16 PST
That is ... most odd.  I don't have a guess for what's up.  Probably an easy fix.
Comment 2 Ali Juma [:ajuma] 2012-01-11 06:47:39 PST
The problem is related to lines 2885-2890 of ipc/ipdl/ipdl/lower.py:

            if switch.nr_cases > 1:
                method.addstmt(switch)
            else:
                method.addstmt(StmtReturn(_Result.NotKnown))

When a protocol has no async messages declared, at the point at which this snippet gets executed (during the generation of Parent.cpp::OnMessageReceived(Message&)) the only case added to |switch| (which is self.asyncSwitch here) is the default case, so |switch| doesn't get added to the method. Then, lines 3530-3536 (which add the SHMEM_CREATED/DESTROYED handlers to self.asyncSwitch) have no effect on the generated method.

Indeed, changing line 2885 to:

            if switch.nr_cases > 0:

fixes the problem (but breaks bug 509581).
Comment 3 Ali Juma [:ajuma] 2012-01-11 10:33:36 PST
An example of a protocol where we have to work around this bug is PCompositor.ipdl in Attachment 587713 [details] [diff], Bug 711168.
Comment 4 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-01-11 12:09:14 PST
OK, that makes sense.  The control flow is way too hard to follow in there.

The simplest fix looks like moving

        # FIXME/bug 535053: only manager protocols and non-manager
        # protocols with union types need Lookup().  we'll give it to
        # all for the time being (simpler)
        if 1 or ptype.isManager():
            self.cls.addstmts(self.implementManagerIface())

before 

        def makeHandlerMethod(name, switch, hasReply, dispatches=0):
            params = [ Decl(Type('Message', const=1, ref=1), msgvar.name) ]

The "right" fix is to rewrite this code again :/.
Comment 5 Ali Juma [:ajuma] 2012-01-12 11:41:58 PST
Created attachment 588131 [details] [diff] [review]
Make IPDL compiler generate code to handle SHMEM-related messages even when there are no other async messages

This implements the fix from Comment 4.
Comment 6 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-01-12 16:53:12 PST
Comment on attachment 588131 [details] [diff] [review]
Make IPDL compiler generate code to handle SHMEM-related messages even when there are no other async messages

It would extremely gross to write a test for this, and not all that incredibly useful IMHO, so r=me without that.
Comment 8 Marco Bonardo [::mak] 2012-01-14 01:34:28 PST
https://hg.mozilla.org/mozilla-central/rev/f9413ba08c26

Note You need to log in before you can comment on or make changes to this bug.