793947 - Race condition in dbg-server.js breaks Marionette

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Jonathan Griffin (:jgriffin)]

There is a race condition in dbg-server.js that is causing Marionette to fail randomly with:

* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "'[JavaScript Error: "this._allowConnection is not a function" {file: "chrome://global/content/devtools/dbg-server.js" line: 272}]' when calling method: [nsIServerSocketListener::onSocketAccepted]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0"  data: yes]

In looking at this a bit, it appears that destroy() is being called before the socket is really closed (the socket will call onStopListening when it closes, which is currently a no-op in dbg-server.js), so there is a window of time when onSocketAccepted will accept a new connection, but this._allowConnection will have been nulled out by destroy().

This, combined with the patch from bug 792867 is breaking Marionette.  There's a fix to the Marionette side of things in bug 793760, but this bug will need to be fixed as well in order to get Marionette operational again.

This is impacting several aspects of test automation for B2G.

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Patch v1

I may have gone a bit too far while trying to avoid leaks. This patch removes the automatic shutdown of the server when the last connection is closed. I think we can live with this, since I still don;t get any mochitest leaks locally. If it doesn't cause any problems with the web console patch we can land it.

Try: https://tbpl.mozilla.org/?tree=Try&rev=fde4b21fd32c

Comment 2

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Patch v2

Forgot to revert the xpcshell test change.

Comment 3

[Mihai Sucan [:msucan]]

Comment on attachment 664865 [details] [diff] [review]
Patch v2

Review of attachment 664865 [details] [diff] [review]:
-----------------------------------------------------------------

Patch looks good and try runs with the web console remoting patches went well.

Comment 4

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/integration/fx-team/rev/225a4da889c5

Comment 5

[Jonathan Griffin (:jgriffin)]

Tim, could we get this merged into m-c soon?  We really need this fix as soon as we can.  Thanks!

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/225a4da889c5