Shut-down of the background hang monitor(bhm) is currently problematic:
- it does not always run until the monitored script-thread does(see
"BackgroundHangMonitor has gone away" mentioned in
https://github.com/servo/servo/issues/34158).
- it shuts-down before the constellation(good, so actually
https://github.com/servo/servo/issues/24850 was "fixed" but in a way
that introduced a new problem), but using a mechanism that allows it to
shutdown before script(the problem above).
- there are various mechanism(see the doc comments removed by this PR)
in place which are meant to ensure a clean shutdown despite the above
problems; those are complicated, and become unnecessary once those
problems are fixed.
All of the above is fixed by the changes in this PR, which ensure the
bhm does not shut-down before script, and also maintains the invariant
that it must shut-down before the constellation(in single-process mode)
or before the main thread(in multi-process mode), but using a mechanism
which allows it to keep running until script shuts-down.
An unnecessary option around the exit signal is also removed.
As a positive side-effect, it also ensures that any script-thread is
shut-down before the constellation(because for the bhm worker to exit,
the monitored script must have exited first), so this should also fix a
host of other problems noted in
https://github.com/servo/servo/issues/30849, but each should be
confirmed independently(and various other improvements seem possible in
their specific contexts, such as joining on script threads, and removing
the `ScriptThreadMessage::ExitScriptThread`).
Fixes: https://github.com/servo/servo/issues/24850 and part of
https://github.com/servo/servo/issues/34158
Testing: Unit tests in `component/background_hang_monitor/tests`. Also
manually tested loading "about-blank" in single- and multi-process mode.
---------
Signed-off-by: gterzian <2792687+gterzian@users.noreply.github.com>
* Remove usage of lazy-static in background_hang_monitor
The lazy-static crate was only used to construct a mutex,
but since Mutex::new is const this can be done at compiletime
instead.
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
* Remove usage of lazy-static in servoshell
Lazy-static was only used to construct a mutex, but
since Mutex::new is const this can simply be done at
compiletime.
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
---------
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
For a long time, `gfx_traits` has held a lot of things unrelated to graphics
and also unrelated to the `gfx` crate (which is mostly about fonts).
This is a cleanup which does a few things:
1. Move non `gfx` crate things out of `gfx_traits`. This is important in
order to prevent dependency cycles with a different integration between
layout, script, and fonts.
2. Rename the `msg` crate to `base`. It didn't really contain anything
to do with messages and instead mostly holds ids, which are used
across many different crates in Servo. This new crate will hold the
*rare* data types that are widely used.
Details:
- All BackgroundHangMonitor-related things from base to a new
`background_hang_monitor_api` crate.
- Moved `TraversalDirection` to `script_traits`
- Moved `Epoch`-related things from `gfx_traits` to `base`.
- Moved `PrintTree` to base. This should be widely useful in Servo.
- Moved `WebrenderApi` from `base` to `webrender_traits` and renamed it
to `WebRenderFontApi`.
> There's a race condition between the reception of
> `BackgroundHangMonitorControlMsg::Exit` and `MonitoredComponentMsg::
> Register`. When the worker receives `Exit`, it stops receiving
> messages, and any remaining messages (including the
> `MonitoredComponentMsg::Register` we sent) in the channel are dropped.
> Wrapping `exit_signal` with this RAII wrapper ensures the exit signal
> is delivered even in such cases.
This should (hopefully) eliminate the intermittent hang-ups in the test
case `test_hang_monitoring_exit_signal` for good.
Bound sampler buffer, add length feedback message, fix intermittence in test
<!-- Please describe your changes on the following line: -->
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix#23109 (GitHub issue number if applicable)
<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___
<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23139)
<!-- Reviewable:end -->
