1772146 - Automatically destroy all DevToolsServer once the last connection drops

Status: RESOLVED FIXED

(DevTools :: Framework, enhancement)

Description

[Alexandre Poirot [:ochameau]]

Bug 1771088 introduced a new "autoDestroy" boolean on DevToolsServer to automatically destroy the server once the last connection drops.
But it appears that it should rather become the default behavior.
Lost of code is already doing this manually in connectors, and some other code isn't doing this whereas it would probably benefit from doing it.

There is only two cases when the server should be kept running:

• when starting the server for --start-debugger-server
• when starting the server for mobile devices
  These two cases will use keepAlive attribute to disable this auto-destroy behavior.

By doing this we will probably more frequently destroy the server when the toolbox closes. Which means that we might have a slight regression when reopening a toolbox a second time. The regression is probably worth the possible memory improvement provided by the destruction of the server.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1772146 - [devtools] Automatically destroy the DevToolsServer once the last connection drops.

Do that except when keepAlive attribute is set to true for server
listening for many incoming client connections.
This is the case when starting server via --start-debugger-server
or when starting the server from the mobile browser.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8fc8b400ab24
[devtools] Automatically destroy the DevToolsServer once the last connection drops. r=jdescottes

Comment 3

[bszekely]

Backed out or causing xpcshell failures on test_register_actor.js

Failure line: TEST-UNEXPECTED-FAIL | devtools/server/tests/xpcshell/test_register_actor.js | xpcshell return code: 0

Push with failures

Failure log

Backout link

[task 2022-06-08T10:45:36.613Z] 10:45:36     INFO -  TEST-PASS | devtools/server/tests/xpcshell/test_shapes_highlighter_helpers.js | took 2959ms
[task 2022-06-08T10:45:36.806Z] 10:45:36     INFO -  TEST-PASS | devtools/server/tests/xpcshell/test_setBreakpoint-on-line-with-multiple-statements.js | took 4169ms
[task 2022-06-08T10:45:36.897Z] 10:45:36     INFO -  TEST-PASS | devtools/server/tests/xpcshell/test_setBreakpoint-on-line-with-multiple-offsets.js | took 4262ms
[task 2022-06-08T10:45:37.227Z] 10:45:37     INFO -  TEST-PASS | devtools/server/tests/xpcshell/test_setBreakpoint-on-line-in-gcd-script.js | took 4652ms
[task 2022-06-08T10:45:37.545Z] 10:45:37     INFO -  TEST-PASS | devtools/server/tests/xpcshell/test_restartFrame-01.js | took 3410ms
[task 2022-06-08T10:45:37.669Z] 10:45:37     INFO -  TEST-PASS | devtools/server/tests/xpcshell/test_connection_closes_all_pools.js | took 1668ms
[task 2022-06-08T10:45:37.777Z] 10:45:37     INFO -  TEST-PASS | devtools/server/tests/xpcshell/test_sessionDataHelpers.js | took 1535ms
[task 2022-06-08T10:45:38.417Z] 10:45:38     INFO -  TEST-PASS | devtools/server/tests/xpcshell/test_webext_apis.js | took 4361ms
[task 2022-06-08T10:45:38.418Z] 10:45:38     INFO -  Retrying tests that failed when run in parallel.
[task 2022-06-08T10:45:38.423Z] 10:45:38     INFO -  TEST-START | devtools/server/tests/xpcshell/test_register_actor.js
[task 2022-06-08T10:45:38.839Z] 10:45:38  WARNING -  TEST-UNEXPECTED-FAIL | devtools/server/tests/xpcshell/test_register_actor.js | xpcshell return code: 0
[task 2022-06-08T10:45:38.839Z] 10:45:38     INFO -  TEST-INFO took 415ms
[task 2022-06-08T10:45:38.839Z] 10:45:38     INFO -  >>>>>>>
[task 2022-06-08T10:45:38.841Z] 10:45:38     INFO -  PID 7138 | [Parent 7138, Main Thread] WARNING: Couldn't get the user appdata directory. Crash events may not be produced.: file /builds/worker/checkouts/gecko/toolkit/crashreporter/nsExceptionHandler.cpp:2972
[task 2022-06-08T10:45:38.841Z] 10:45:38     INFO -  (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2022-06-08T10:45:38.841Z] 10:45:38     INFO -  (xpcshell/head.js) | test run_next_test 0 pending (2)
[task 2022-06-08T10:45:38.841Z] 10:45:38     INFO -  (xpcshell/head.js) | test MAIN run_test finished (2)
[task 2022-06-08T10:45:38.841Z] 10:45:38     INFO -  running event loop
[task 2022-06-08T10:45:38.841Z] 10:45:38     INFO -  devtools/server/tests/xpcshell/test_register_actor.js | Starting test_lazy_api
[task 2022-06-08T10:45:38.841Z] 10:45:38     INFO -  (xpcshell/head.js) | test test_lazy_api pending (2)
[task 2022-06-08T10:45:38.842Z] 10:45:38     INFO -  TEST-PASS | devtools/server/tests/xpcshell/test_register_actor.js | test_lazy_api - [test_lazy_api : 35] true == true
[task 2022-06-08T10:45:38.842Z] 10:45:38     INFO -  TEST-PASS | devtools/server/tests/xpcshell/test_register_actor.js | test_lazy_api - [test_lazy_api : 38] true == true
[task 2022-06-08T10:45:38.842Z] 10:45:38     INFO -  TEST-PASS | devtools/server/tests/xpcshell/test_register_actor.js | test_lazy_api - [test_lazy_api : 39] true == true
[task 2022-06-08T10:45:38.842Z] 10:45:38     INFO -  TEST-PASS | devtools/server/tests/xpcshell/test_register_actor.js | test_lazy_api - [test_lazy_api : 40] true == true
[task 2022-06-08T10:45:38.843Z] 10:45:38     INFO -  (xpcshell/head.js) | test run_next_test 0 finished (2)

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2140f152848d
[devtools] Automatically destroy the DevToolsServer once the last connection drops. r=jdescottes

Comment 5

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/2140f152848d

Comment 6

[Julian Descottes [:jdescottes]]

== Change summary for alert #34373 (as of Thu, 09 Jun 2022 19:05:56 GMT) ==

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)
207%	target:parent-process objects-with-no-stacks	linux1804-64-qr		12.12 -> 37.17
193%	target:parent-process objects-with-no-stacks	linux1804-64-shippable-qr		12.04 -> 35.33

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
16%	reload-debugger:content-process objects-with-stacks	linux1804-64-qr		1,145.25 -> 961.25
16%	reload-debugger:content-process objects-with-stacks	linux1804-64-shippable-qr		1,150.00 -> 966.00

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=34373

Comment 7

[Alexandre Poirot [:ochameau]]

All the intermittents seems to be related to failure in connecting to the test server:
https://searchfox.org/mozilla-central/rev/170f06a720ddabee44c728b05ad30b18b066acca/devtools/client/framework/browser-toolbox/test/helpers-browser-toolbox.js#88-103
We see the Has session state message, but not the Got transport.

I tried enabling the headless to speed up test execution, but it only reduce the number of occurences:
https://treeherder.mozilla.org/jobs?repo=try&revision=e5b146cc9a8e8124ce9f36ae5310f2aafb1e6398
Enabling keepAlive on the Browser Toolbox DevToolsServer doesn't change anything:
https://treeherder.mozilla.org/jobs?repo=try&revision=ba379cb1b256892e4ba3bdf9c5c2433071f3b6fb

I'll try by enabling keepAlive on the test server.

Comment 8

[Julian Descottes [:jdescottes]]

FWIW, failures seem to be windows only.

I'll try by enabling keepAlive on the test server.

Since we are fixing the port for this server, maybe it's important to avoid destroying and recreating it too often, in case the port has not been released?