Closed Bug 792867 Opened 7 years ago Closed 7 years ago

Debugger mochitests leak when run separately

Categories

(DevTools :: Debugger, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 18

People

(Reporter: past, Assigned: past)

References

Details

Attachments

(1 file, 2 obsolete files)

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.
Attached patch Patch v1 (obsolete) — Splinter Review
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.
Attached patch Patch v2 (obsolete) — Splinter Review
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+
Attached patch Patch v3Splinter Review
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
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 18
Depends on: 793947
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.