Debugger mochitests leak when run separately

RESOLVED FIXED in Firefox 18

Status

DevTools
Debugger
P2
normal
RESOLVED FIXED
6 years ago
7 days ago

People

(Reporter: past, Assigned: past)

Tracking

Trunk
Firefox 18

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Debugger mochitests currently leak if you run them one by one, or as a set, but not if they run as part of the whole mochitest-other suite. We should fix this. 

Mihai made the terrific diagnosis that DH_allowConnection holds on to various DOMWindows. I'm testing a patch with more extensive changes right now.
(Assignee)

Comment 1

6 years ago
Created attachment 663083 [details] [diff] [review]
Patch v1

This fixes the leaks, but 2 tests are broken. The bug is probably in the tests and I hope to have it figured out tomorrow.
(Assignee)

Comment 2

6 years ago
Created attachment 663427 [details] [diff] [review]
Patch v2

This version fixes all the leaks. The culprit was the _allowConnection handler as Mihai discovered, so we now make sure to clear it on DebuggerServer.destroy(). After adding a few more cleanups there, it became important to carefully coordinate shutdown and opening in the notification box that appears when the user tries to open another debugger in a second tab (or window). We now wait for the Debugger:Shutdown event before opening the second debugger, so everything works as before.

Try: https://tbpl.mozilla.org/?tree=Try&rev=deaca566213b
Attachment #663083 - Attachment is obsolete: true
Attachment #663427 - Flags: review?(rcampbell)
Comment on attachment 663427 [details] [diff] [review]
Patch v2

 function run_test()
 {
   // Allow incoming connections.
-  DebuggerServer.init(function () { return true; });
+  DebuggerServer.init(function () true);


can you get that down to just 3 characters? :)


I wish I had something funnier to say here. R+.
Attachment #663427 - Flags: review?(rcampbell) → review+
(Assignee)

Comment 4

6 years ago
Created attachment 663707 [details] [diff] [review]
Patch v3

Last try push had a couple of oranges in the modified unit test, so I made it more robust. The same issue with restarting the server after it shuts down that I mentioned in comment 2. Try run was clean this time:
https://tbpl.mozilla.org/?tree=Try&rev=5c7fe9ff1b99
Attachment #663427 - Attachment is obsolete: true
Attachment #663707 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/4e6eb2cb00b5
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 18
Depends on: 793947

Updated

7 days ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.