Closed Bug 1564168 Opened 5 years ago Closed 5 years ago

[jsdbg2] Debugger.prototype.enabled should be removed

Categories

(Core :: JavaScript Engine, task, P3)

Product:
Core
Component:
JavaScript Engine
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla70
Tracking Flags:
Tracking Status
firefox70 --- fixed

People

(Reporter: jimb, Assigned: jimb)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [debugger-mvp])

Attachments

(2 files, 2 obsolete files)

Bug 1564168 - [jsdbg2] Debugger.prototype.enabled should be removed (part 1). r=jimb
(deleted), text/x-phabricator-request
Details
Bug 1564168 - [jsdbg2] Debugger.prototype.enabled should be removed. r=jimb
(deleted), text/x-phabricator-request
Details

In the Debugger API, every Debugger object has an enabled property, to which one can assign true or false to enable or disable the debugger. The idea was that, to make closing the debugger panel more robust, Debugger should have a single cut-off point guaranteed to prevent it from interfering with further execution or consuming any resources.

However, this turned out to be more complicated than expected. Two examples:

  1. Even if a Debugger has hooks set that would prevent it from being GC'd (because the hooks may cause observable side effects), if the Debugger is disabled and not reachable from JavaScript, then it's okay to GC it anyway, since none of those hooks will ever fire. The GC logic actually has a special case for this; maybe it's correct? But who cares?

  2. When there are multiple Debuggers observing a realm, our hooks often construct a list of Debuggers to notify about some interesting event, and then iterate over that list. But Debuggers earlier in the list can disable those later in the list. We've forgotten to handle this case several times.

But in the end, the enabled property isn't really necessary: just calling removeAllDebuggees, or some custom method that does that along with some other debuggee-independent tear down, should be just as useful, and uses only functionality that we have to implement anyway for other reasons (like removing debuggees). The only advantage of the enabled property is that, if you set it back to true, you get back the original set of debuggees; that's kind of nice, but not important enough to justify the complexity it ends up causing.

Priority: -- → P3
Blocks: dbg-70
Whiteboard: [debugger-mvp]
Assignee: nobody → jlaster
Status: NEW → ASSIGNED
Pushed by jlaster@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1ebcbc3c2446 [jsdbg2] Debugger.prototype.enabled should be removed (part 1). r=jimb

https://hg.mozilla.org/mozilla-central/rev/1ebcbc3c2446

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox70: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Pushed by jlaster@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bf0aa9a2c384 [jsdbg2] Debugger.prototype.enabled should be removed. r=jimb
Pushed by jlaster@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/34e9a64cc887 [jsdbg2] Debugger.prototype.enabled should be removed. r=jimb

Backed out changeset 34e9a64cc887 (bug 1564168) for chrome failures at js/xpconnect/tests/chrome/test_precisegc.xul

Backout https://hg.mozilla.org/integration/autoland/rev/78c2b099971159fdefb85680435e21b46a2398d2

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=34e9a64cc8875d33369d764d3574b26a959ade9e

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=257467552&repo=autoland&lineNumber=9961

21:47:55 INFO - TEST-START | js/xpconnect/tests/chrome/test_precisegc.xul
21:47:55 INFO - GECKO(1847) | ++DOMWINDOW == 277 (0x129859400) [pid = 1847] [serial = 412] [outer = 0x12bfb92e0]
21:47:56 INFO - GECKO(1847) | MEMORY STAT | vsize 7720MB | residentFast 461MB | heapAllocated 162MB
21:47:56 INFO - GECKO(1847) | 2019-07-19 21:47:56.550 firefox[1847:16109] Persistent UI failed to open file file:///Users/cltbld/Library/Saved%20Application%20State/org.mozilla.nightlydebug.savedState/window_1.data: No such file or directory (2)
21:47:56 INFO - GECKO(1847) | --DOCSHELL 0x122fc7000 == 34 [pid = 1847] [id = {a6a75488-b5a0-1e49-bc16-b6f83f2a3ddd}] [url = http://example.org/tests/js/xpconnect/tests/mochitest/file_documentdomain.html]
21:47:56 INFO - GECKO(1847) | --DOCSHELL 0x122fc6800 == 33 [pid = 1847] [id = {f75dac00-03f3-7848-bf85-1b153ce71b0f}] [url = http://test2.example.org/tests/js/xpconnect/tests/mochitest/file_documentdomain.html]
21:47:56 INFO - GECKO(1847) | --DOCSHELL 0x122fc6000 == 32 [pid = 1847] [id = {2caf0a3c-8845-ad47-9b42-b7691478de6e}] [url = http://test1.example.org/tests/js/xpconnect/tests/mochitest/file_documentdomain.html]
21:47:56 INFO - GECKO(1847) | --DOCSHELL 0x122cc1000 == 31 [pid = 1847] [id = {118832a5-3a69-774d-b07f-5b7b0356d479}] [url = http://test1.example.org/tests/js/xpconnect/tests/mochitest/file_documentdomain.html]
21:47:56 INFO - GECKO(1847) | Assertion failure: !tc->isMarkedGray(), at /builds/worker/workspace/build/src/js/src/gc/GC.cpp:9250
21:47:56 INFO - TEST-INFO | Main app process: exit 1
21:47:56 INFO - Buffered messages logged at 21:47:56
21:47:56 INFO - TEST-PASS | js/xpconnect/tests/chrome/test_precisegc.xul | callback executed
21:47:56 INFO - Buffered messages finished
21:47:56 ERROR - TEST-UNEXPECTED-FAIL | js/xpconnect/tests/chrome/test_precisegc.xul | application terminated with exit code 1
21:47:56 INFO - runtests.py | Application ran for: 0:00:31.210499
21:47:56 INFO - zombiecheck | Reading PID log: /var/folders/cs/c_r718x51b77c0f_b_bpn_d0000017/T/tmpntmh7wpidlog
21:47:56 INFO - mozcrash Copy/paste: /Users/cltbld/tasks/task_1563569029/build/macosx64-minidump_stackwalk /var/folders/cs/c_r718x51b77c0f_b_bpn_d0000017/T/tmpZWemCK.mozrunner/minidumps/518E826F-8DA8-46CD-ADB3-E71B370F3F73.dmp /Users/cltbld/tasks/task_1563569029/build/symbols
21:48:03 INFO - mozcrash Saved minidump as /Users/cltbld/tasks/task_1563569029/build/blobber_upload_dir/518E826F-8DA8-46CD-ADB3-E71B370F3F73.dmp
21:48:03 INFO - mozcrash Saved app info as /Users/cltbld/tasks/task_1563569029/build/blobber_upload_dir/518E826F-8DA8-46CD-ADB3-E71B370F3F73.extra
21:48:03 INFO - PROCESS-CRASH | js/xpconnect/tests/chrome/test_precisegc.xul | application crashed [@ js::gc::detail::AssertCellIsNotGray(js::gc::Cell const*)]

Attachment #9079860 - Attachment is obsolete: true
Pushed by jlaster@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d025c39278a3 [jsdbg2] Debugger.prototype.enabled should be removed. r=jimb
Attachment #9077473 - Attachment is obsolete: true
Pushed by jlaster@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2c5e99c6de6a [jsdbg2] Debugger.prototype.enabled should be removed.
Pushed by jlaster@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bb8ed79d24e4 [jsdbg2] Debugger.prototype.enabled should be removed.
Backout by rgurzau@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8daa185ec414 Backed out changeset bb8ed79d24e4 for assertion failures at GC.cpp on a CLOSED TREE.

Backed out changeset bb8ed79d24e4 (bug 1564168) for assertion failures at GC.cpp on a CLOSED TREE.

Backout link: https://hg.mozilla.org/integration/autoland/rev/8daa185ec414e0b11f409985e655444034738e7d

**Push with failures:**https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=bb8ed79d24e4582fea57ec92d1cb728d2f6321a6&selectedJob=259240356

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=259240356&repo=autoland&lineNumber=1706

Log snippet:
[task 2019-07-31T18:57:06.018Z] 18:57:06 INFO - TEST-INFO took 628ms
[task 2019-07-31T18:57:06.045Z] 18:57:06 INFO - TEST-OK | js/xpconnect/tests/chrome/test_onGarbageCollection.html
[task 2019-07-31T18:57:06.052Z] 18:57:06 INFO - GECKO(1776) | ++DOMWINDOW == 167 (1E7DD000) [pid = 3520] [serial = 184] [outer = 25A64CA0]
[task 2019-07-31T18:57:06.119Z] 18:57:06 INFO - TEST-START | js/xpconnect/tests/chrome/test_precisegc.xul
[task 2019-07-31T18:57:06.138Z] 18:57:06 INFO - GECKO(1776) | ++DOMWINDOW == 168 (22CB3C00) [pid = 3520] [serial = 185] [outer = 25A64CA0]
[task 2019-07-31T18:57:06.581Z] 18:57:06 INFO - GECKO(1776) | MEMORY STAT | vsize 823MB | vsizeMaxContiguous 733MB | residentFast 280MB | heapAllocated 114MB
[task 2019-07-31T18:57:06.604Z] 18:57:06 INFO - GECKO(1776) | --DOCSHELL 24769400 == 28 [pid = 3520] [id = {47e99b03-321b-4db4-852c-43b41b566d59}] [url = http://mochi.test:8888/tests/js/xpconnect/tests/mochitest/file_nodelists.html]
[task 2019-07-31T18:57:06.604Z] 18:57:06 INFO - GECKO(1776) | Assertion failure: !tc->isMarkedGray(), at z:/build/build/src/js/src/gc/GC.cpp:9315
[task 2019-07-31T18:57:06.884Z] 18:57:06 INFO - TEST-INFO | Main app process: exit 1
[task 2019-07-31T18:57:06.884Z] 18:57:06 INFO - Buffered messages logged at 18:57:06
[task 2019-07-31T18:57:06.885Z] 18:57:06 INFO - TEST-PASS | js/xpconnect/tests/chrome/test_precisegc.xul | callback executed
[task 2019-07-31T18:57:06.885Z] 18:57:06 INFO - Buffered messages finished
[task 2019-07-31T18:57:06.885Z] 18:57:06 ERROR - TEST-UNEXPECTED-FAIL | js/xpconnect/tests/chrome/test_precisegc.xul | application terminated with exit code 1
[task 2019-07-31T18:57:06.885Z] 18:57:06 INFO - runtests.py | Application ran for: 0:00:18.008000
[task 2019-07-31T18:57:06.885Z] 18:57:06 INFO - zombiecheck | Reading PID log: c:\users\task_1564593383\appdata\local\temp\tmplpdwhspidlog
[task 2019-07-31T18:57:06.888Z] 18:57:06 INFO - mozcrash Copy/paste: Z:\task_1564593383\build\win32-minidump_stackwalk.exe c:\users\task_1564593383\appdata\local\temp\tmp0vlgbg.mozrunner\minidumps\fc0276e3-1791-40cb-b523-85c1a68616ec.dmp Z:\task_1564593383\build\symbols
[task 2019-07-31T18:57:26.603Z] 18:57:26 INFO - mozcrash Saved minidump as Z:\task_1564593383\build\blobber_upload_dir\fc0276e3-1791-40cb-b523-85c1a68616ec.dmp
[task 2019-07-31T18:57:26.603Z] 18:57:26 INFO - mozcrash Saved app info as Z:\task_1564593383\build\blobber_upload_dir\fc0276e3-1791-40cb-b523-85c1a68616ec.extra
[task 2019-07-31T18:57:26.762Z] 18:57:26 INFO - PROCESS-CRASH | js/xpconnect/tests/chrome/test_precisegc.xul | application crashed [@ js::gc::detail::AssertCellIsNotGray(js::gc::Cell const *)]
[task 2019-07-31T18:57:26.762Z] 18:57:26 INFO - Crash dump filename: c:\users\task_1564593383\appdata\local\temp\tmp0vlgbg.mozrunner\minidumps\fc0276e3-1791-40cb-b523-85c1a68616ec.dmp
[task 2019-07-31T18:57:26.762Z] 18:57:26 INFO - Operating system: Windows NT
[task 2019-07-31T18:57:26.762Z] 18:57:26 INFO - 6.1.7601 Service Pack 1
[task 2019-07-31T18:57:26.762Z] 18:57:26 INFO - CPU: x86
[task 2019-07-31T18:57:26.762Z] 18:57:26 INFO - GenuineIntel family 6 model 63 stepping 2
[task 2019-07-31T18:57:26.763Z] 18:57:26 INFO - 8 CPUs
[task 2019-07-31T18:57:26.763Z] 18:57:26 INFO -
[task 2019-07-31T18:57:26.763Z] 18:57:26 INFO - GPU: UNKNOWN
[task 2019-07-31T18:57:26.763Z] 18:57:26 INFO -
[task 2019-07-31T18:57:26.763Z] 18:57:26 INFO - Crash reason: EXCEPTION_BREAKPOINT
[task 2019-07-31T18:57:26.763Z] 18:57:26 INFO - Crash address: 0x5b802d9b
[task 2019-07-31T18:57:26.763Z] 18:57:26 INFO - Process uptime: 18 seconds
[task 2019-07-31T18:57:26.763Z] 18:57:26 INFO -
[task 2019-07-31T18:57:26.763Z] 18:57:26 INFO - Thread 0 (crashed)
[task 2019-07-31T18:57:26.763Z] 18:57:26 INFO - 0 xul.dll!js::gc::detail::AssertCellIsNotGray(js::gc::Cell const *) [GC.cpp:bb8ed79d24e4582fea57ec92d1cb728d2f6321a6 : 9315 + 0x0]
[task 2019-07-31T18:57:26.764Z] 18:57:26 INFO - eip = 0x5b802d9b esp = 0x0037ec90 ebp = 0x0037eca0 ebx = 0x0220a800
[task 2019-07-31T18:57:26.764Z] 18:57:26 INFO - esi = 0x18983790 edi = 0x0220c000 eax = 0x69b3267c ecx = 0x00002463
[task 2019-07-31T18:57:26.764Z] 18:57:26 INFO - edx = 0x6885e340 efl = 0x00000216
[task 2019-07-31T18:57:26.764Z] 18:57:26 INFO - Found by: given as instruction pointer in context

Flags: needinfo?(jlaster)
Pushed by jlaster@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/781d092114da [jsdbg2] Debugger.prototype.enabled should be removed.
Assignee: jlaster → jimb
Pushed by lsmyth@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e40f0edf3e40 [jsdbg2] Debugger.prototype.enabled should be removed.

https://hg.mozilla.org/mozilla-central/rev/e40f0edf3e40

Status: REOPENED → RESOLVED
Closed: 5 years ago → 5 years ago
status-firefox70: affected → fixed
Resolution: --- → FIXED
Flags: needinfo?(jlaster)
Regressions: 1581530
No longer regressions: 1581530
Regressions: 1590589
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: