Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[manifest] Explicitly terminate the multiprocessing.Pool #27175

Merged
merged 1 commit into from Jan 14, 2021

Conversation

stephenmcgruer
Copy link
Contributor

@stephenmcgruer stephenmcgruer commented Jan 13, 2021

Python requires that a Pool object be terminated before it is GC'd
(https://docs.python.org/3/library/multiprocessing.html#multiprocessing.pool.Pool):

Warning: multiprocessing.pool objects have internal resources that need
to be properly managed (like any other resource) by using the pool as a
context manager or by calling close() and terminate() manually. Failure
to do this can lead to the process hanging on finalization.

Note that is not correct to rely on the garbage colletor to destroy the
pool as CPython does not assure that the finalizer of the pool will be
called (see object.__del__() for more information).

Chromium has been seeing hangs in 'wpt manifest' invocations since
moving to Python 3 for WPT, which sounds a lot like 'Failure to do this
can lead to the process hanging on finalization.' So let's
terminate our Pool properly :)

Python requires that a Pool object be terminated before it is GC'd
(https://docs.python.org/3/library/multiprocessing.html#multiprocessing.pool.Pool):

```
Warning: multiprocessing.pool objects have internal resources that need
to be properly managed (like any other resource) by using the pool as a
context manager or by calling close() and terminate() manually. Failure
to do this can lead to the process hanging on finalization.

Note that is not correct to rely on the garbage colletor to destroy the
pool as CPython does not assure that the finalizer of the pool will be
called (see object.__del__() for more information).
```

Chromium has been seeing hangs in 'wpt manifest' invocations since
moving to Python 3 for WPT, which sounds a lot like 'Failure to do this
can lead to the process hanging on finalization.' So let's
terminate our Pool properly :)
@stephenmcgruer stephenmcgruer changed the title [manifest] Explicitly context-manage the Pool [manifest] Explicitly terminate the multiprocessing.Pool Jan 13, 2021
@jgraham jgraham merged commit eb6aa97 into master Jan 14, 2021
@jgraham jgraham deleted the smcgruer/wptmanifest-pool branch January 14, 2021 14:21
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
This rolls up to WPT SHA f333959abe07e6c4cfb388f4f7a03a0a109b38a5

Importantly, it brings in
web-platform-tests/wpt#27175 which should allow
us to move 'wpt manifest' calls back to Python 3. (This CL does not make
that switch, it is just rolling tools).

It also brings in the change that makes '--py3' the default mode
for 'wpt'. Since the wptrunner bots aren't ready for that yet, I also
updated testing/scripts/{run_android_wpt.py, run_wpt_tests.py} to
pass --py2 to 'wpt'.

Bug: 1161274, 1166741

Cq-Include-Trybots: luci.chromium.try:linux-wpt-identity-fyi-rel,linux-wpt-input-fyi-rel
Change-Id: I0978d0685f2ef387e22e87e433d4a191e09e7597
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2627882
Reviewed-by: Luke Z <lpz@chromium.org>
Reviewed-by: Stephen Martinis <martiniss@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843760}
GitOrigin-RevId: 193b96b16836f33e1782eedc69855f76dcd4e51a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants