Use toolkit.asyncshutdown.crash_timout as shutdown monitor base value
Categories
(Testing :: geckodriver, enhancement, P3)
Tracking
(Not tracked)
People
(Reporter: ato, Unassigned, Mentored)
References
Details
(Whiteboard: [lang=rust])
geckodriver is currently hard-coded with a 70 second wait before forcefully killing the Firefox process. This is adequate for regular optimised builds, but the base value for ASAN is longer, and the value can be overridden using the toolkit.asyncshutdown.crash_timeout preference. Unfortunately mozprofile does not yet have a way of presenting the union of the user.js and prefs.js files, i.e. the effective pref value.
|
||
Comment 1•6 years ago
|
||
Here a link to the appropriate definitions: https://dxr.mozilla.org/mozilla-central/source/modules/libpref/init/all.js#1071-1077 (In reply to Andreas Tolfsen ‹:ato› from comment #0) > Unfortunately mozprofile does not yet have a way of presenting the > union of the user.js and prefs.js files, i.e. the effective pref value. Note that whatever is in user.js will automatically be added by Firefox to prefs.js each time the browser starts. Means when a user modifies the value it will be reset with the next start/restart.
|
Reporter | |
Comment 2•6 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #1) > (In reply to Andreas Tolfsen ‹:ato› from comment #0) > > > Unfortunately mozprofile does not yet have a way of presenting > > the union of the user.js and prefs.js files, i.e. the effective > > pref value. > > Note that whatever is in user.js will automatically be added > by Firefox to prefs.js each time the browser starts. Means > when a user modifies the value it will be reset with the next > start/restart. OK, that’s useful. However, mozprofile only lets you read the file once when it is initialised because there initially was no use case for reading the prefs file multiple times. So some changes to mozprofile are required.
|
|
Reporter | |
Updated•6 years ago
|
|
|
||
Comment 3•6 years ago
|
||
Please note that we should not actually change this value, but retrieve it and make use of it in geckodriver. ASAN builds will take longer for shutdown due to various reasons. See the explanation from Andrew in bug 1472853 comment 9. As such we have to find a way to send the information. I would propose that we use a vendor-prefixed new session parameter like `moz:shutdownTimeout`.
|
|
||
Comment 4•5 years ago
|
||
Andreas, I had a quick look but I'm actually struggling where to add the code to retrieve the timeout value from the returned capabilities. Also should the default timeout of 70s defined as property for MarionetteHandler? While MarionetteSettings sounds better it looks like that this struct is only used for input settings.
I would kinda like to get this fixed to reduce the amount of possible unwanted forced-kills of Firefox for ASAN jobs.
|
|
Reporter | |
Comment 5•5 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #4)
Andreas, I had a quick look but I'm actually struggling where
to add the code to retrieve the timeout value from the returned
capabilities.
You need to have a mutable base timeout (probably leave it at 70?)
which you overwrite with the value from moz:shutdownTimeout once
the session has been established.
We handle responses over in MarionetteSession::response():
https://searchfox.org/mozilla-central/rev/5053031ba7621fa8f63f42de4c204ab3561e4e59/testing/geckodriver/src/marionette.rs#690
This is where you need to extract moz:shutdownTimeout.
Also should the default timeout of 70s defined as
property forMarionetteHandler? WhileMarionetteSettings
sounds better it looks like that this struct is only used for
input settings.
Yes that sounds fine. It’s a configurable user setting.
|
|
||
Comment 6•3 years ago
|
||
So the hard-coded 70s shutdown time can be found here:
It's part of MarionetteHandler::delete_session() that gets a Session object. As Andreas pointed out above the capabilities as part of the WebDriver:NewSession command are handled here:
That means that within delete_session() we can simply try to retrieve moz:shutdownTimeout and use it, or fallback to the hardcoded 70s.
|
||
Updated•2 years ago
|
|
|
||
Comment 7•4 months ago
|
||
The code as referenced in my last comment has been moved and is now available at:
https://searchfox.org/mozilla-central/rev/91cc8848427fdbbeb324e6ca56a0d08d32d3c308/testing/geckodriver/src/browser.rs#157
Description
•