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 :)

@wpt-pr-bot wpt-pr-bot requested review from jgraham and LukeZielinski Jan 13, 2021
@stephenmcgruer stephenmcgruer force-pushed the smcgruer/wptmanifest-pool branch from b80fbbf to 9df5a58 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 :)
@stephenmcgruer stephenmcgruer force-pushed the smcgruer/wptmanifest-pool branch from 9df5a58 to eae6a51 Jan 13, 2021
@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
48 checks passed
48 checks passed
update-pr-preview
Details
update-pr-preview
Details
update-pr-preview
Details
build-and-publish
Details
build-and-publish
Details
build-and-tag
Details
build-and-tag
Details
Azure Pipelines Build #20210114.39 succeeded
Details
Azure Pipelines (./wpt test-jobs) ./wpt test-jobs succeeded
Details
Azure Pipelines (affected tests without changes: Safari Technology Preview) affected tests without changes: Safari Technology Preview succeeded
Details
Azure Pipelines (affected tests: Safari Technology Preview) affected tests: Safari Technology Preview succeeded
Details
Azure Pipelines (infrastructure/ tests: macOS) infrastructure/ tests: macOS succeeded
Details
Azure Pipelines (tools/ unittests: Windows + Python 3.6) tools/ unittests: Windows + Python 3.6 succeeded
Details
Azure Pipelines (tools/ unittests: Windows + Python 3.8) tools/ unittests: Windows + Python 3.8 succeeded
Details
Azure Pipelines (tools/ unittests: Windows) tools/ unittests: Windows succeeded
Details
Azure Pipelines (tools/ unittests: macOS + Python 3.6) tools/ unittests: macOS + Python 3.6 succeeded
Details
Azure Pipelines (tools/ unittests: macOS + Python 3.8) tools/ unittests: macOS + Python 3.8 succeeded
Details
Azure Pipelines (tools/ unittests: macOS) tools/ unittests: macOS succeeded
Details
Azure Pipelines (tools/wpt/ tests: Windows + Python 3.6) tools/wpt/ tests: Windows + Python 3.6 succeeded
Details
Azure Pipelines (tools/wpt/ tests: Windows + Python 3.8) tools/wpt/ tests: Windows + Python 3.8 succeeded
Details
Azure Pipelines (tools/wpt/ tests: Windows) tools/wpt/ tests: Windows succeeded
Details
Azure Pipelines (tools/wpt/ tests: macOS + Python 3.6) tools/wpt/ tests: macOS + Python 3.6 succeeded
Details
Azure Pipelines (tools/wpt/ tests: macOS + Python 3.8) tools/wpt/ tests: macOS + Python 3.8 succeeded
Details
Azure Pipelines (tools/wpt/ tests: macOS) tools/wpt/ tests: macOS succeeded
Details
Azure Pipelines (tools/wptrunner/ unittests: Windows + Python 3.6) tools/wptrunner/ unittests: Windows + Python 3.6 succeeded
Details
Azure Pipelines (tools/wptrunner/ unittests: Windows + Python 3.8) tools/wptrunner/ unittests: Windows + Python 3.8 succeeded
Details
Azure Pipelines (tools/wptrunner/ unittests: Windows) tools/wptrunner/ unittests: Windows succeeded
Details
Azure Pipelines (tools/wptrunner/ unittests: macOS + Python 3.6) tools/wptrunner/ unittests: macOS + Python 3.6 succeeded
Details
Azure Pipelines (tools/wptrunner/ unittests: macOS + Python 3.8) tools/wptrunner/ unittests: macOS + Python 3.8 succeeded
Details
Azure Pipelines (tools/wptrunner/ unittests: macOS) tools/wptrunner/ unittests: macOS succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests) wpt.fyi hook: safari-preview-affected-tests succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests-without-changes) wpt.fyi hook: safari-preview-affected-tests-without-changes succeeded
Details
download-firefox-nightly Community-TC (pull_request)
Details
infrastructure/ tests Community-TC (pull_request)
Details
infrastructure/ tests (Python 3) Community-TC (pull_request)
Details
lint Community-TC (pull_request)
Details
resources/ tests (Python 2) Community-TC (pull_request)
Details
resources/ tests (Python 3.6) Community-TC (pull_request)
Details
resources/ tests (Python 3.8) Community-TC (pull_request)
Details
sink-task Community-TC (pull_request)
Details
tools/ integration tests (Python 2) Community-TC (pull_request)
Details
tools/ integration tests (Python 3.6) Community-TC (pull_request)
Details
tools/ integration tests (Python 3.8) Community-TC (pull_request)
Details
tools/ unittests (Python 2) Community-TC (pull_request)
Details
tools/ unittests (Python 3.6) Community-TC (pull_request)
Details
tools/ unittests (Python 3.8) Community-TC (pull_request)
Details
wpt-decision-task Community-TC (pull_request)
Details
wpt.fyi - safari[experimental] Safari results
Details
@jgraham jgraham deleted the smcgruer/wptmanifest-pool branch Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants