Bug 1509362 - Don't crash when constructing actor during content shutdown, r=jld
☠☠ backed out by 9bfe29337ffe ☠ ☠
authorNika Layzell <nika@thelayzells.com>
Thu, 22 Nov 2018 13:40:32 -0500
changeset 507663 c3fe435e473a463fbc22d4afa531bdedb757079c
parent 507662 6494840edc17b976778f057b1edd0b7c03e19d0b
child 507664 2d672bed9a1db1051426642e304e8ec7ee21c8d3
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjld
bugs1509362
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1509362 - Don't crash when constructing actor during content shutdown, r=jld When shutting down a content process, we call `Close` on the `IToplevelProtocol`. This causes the MessageChannel to be `Close`-ed, which in turn sends a `GOODBYE_MESSAGE`: https://searchfox.org/mozilla-central/rev/876022232b15425bb9efde189caf747823b39567/ipc/glue/MessageChannel.cpp#2852 This message is intercepted on the I/O thread in the content process, before any code is informed in content, and used to set the `mChannelState` property to `ChannelClosing`: https://searchfox.org/mozilla-central/rev/876022232b15425bb9efde189caf747823b39567/ipc/glue/MessageChannel.cpp#1176 Once this state has been set, which is performed as soon as the message is received, whether or not other messages have been processed yet, no messages can be sent back to the parent process. This is usually what causes the 'Too late to send/recv' message spam in the console, as we're still trying to send messages at this time. Usually this is fine - the message send fails, but we gracefully recover, and the process begins shutting down like normal. Unfortunately, child actor constructors currently have code automatically generated in them which causes a process crash if the send fails. As it's impossible for the main thread to know that the channel has been closed ahead of time (due to this happening out-of-band), we can then cause random content process crashes during shutdown due to actor construction. Unfortunately, we can't just destroy the actor, as our caller may (and often do) depend on the actor reference they gave us still being valid after calling Send*Constructor. Fortunately, if a message send failed, it means we're in the process of being shut down. This patch handles this by ignoring ctor send errors, and treating them like messages which successfully were queued to send, but got lost due to the other side hanging up. The actor will be gracefully destroyed in DestroySubtree when its manager is destroyed. Differential Revision: https://phabricator.services.mozilla.com/D12695
ipc/ipdl/ipdl/lower.py
--- a/ipc/ipdl/ipdl/lower.py
+++ b/ipc/ipdl/ipdl/lower.py
@@ -3890,23 +3890,45 @@ class _GenerateProtocolActorCode(ipdl.as
 
     def genAsyncCtor(self, md):
         actor = md.actorDecl()
         method = MethodDefn(self.makeSendMethodDecl(md))
         method.addstmts(self.ctorPrologue(md) + [Whitespace.NL])
 
         msgvar, stmts = self.makeMessage(md, errfnSendCtor)
         sendok, sendstmts = self.sendAsync(md, msgvar)
+
+        warnif = StmtIf(ExprNot(sendok))
+        warnif.addifstmt(_printWarningMessage('Error sending constructor'))
+
         method.addstmts(
+            # Build our constructor message & verify it.
             stmts
             + self.genVerifyMessage(md.decl.type.verify, md.params,
                                     errfnSendCtor, ExprVar('msg__'))
+
+            # Assert that we haven't been shut down yet.
+            + [_abortIfFalse(ExprCall(ExprVar('IPCOpen')),
+                             "Cannot send constructor after ActorDestroy")]
+
+            # Notify the other side about the newly created actor.
+            #
+            # If the MessageChannel is closing, and we haven't been told yet,
+            # this send may fail. This error is ignored to treat it like a
+            # message being lost due to the other side shutting down before
+            # processing it.
+            #
+            # NOTE: We don't free the actor here, as our caller may be
+            # depending on it being alive after calling SendConstructor.
             + sendstmts
-            + self.failCtorIf(md, ExprNot(sendok))
-            + [StmtReturn(actor.var())])
+
+            # Warn if the message failed to send, and return our newly created
+            # actor.
+            + [warnif,
+               StmtReturn(actor.var())])
 
         lbl = CaseLabel(md.pqReplyId())
         case = StmtBlock()
         case.addstmt(StmtReturn(_Result.Processed))
         # TODO not really sure what to do with async ctor "replies" yet.
         # destroy actor if there was an error?  tricky ...
 
         return method, (lbl, case)