GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-113205: test_multiprocessing.test_terminate: Test the API on threadpools by encukou · Pull Request #114186 · python/cpython

Conversation

@encukou

@encukou encukou commented Jan 17, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Threads can't be forced to terminate (without potentially corrupting too much state), so the expected behaviour of ThreadPool.terminate is to wait for the currently executing tasks to finish.

The entire terminate test was skipped in GH-110848 (0e9c364). Instead of skipping it entirely, we should ensure the API eventually succeeds.

For the record: on my machine, when the test is un-skipped on 3.12, the task manages to start in about 1.5% cases.


On older branches, the test is not skipped, so it occasionally takes 5 minutes. The backports will fix issue #113205.

  • Issue: Occasionally multiprocessing.test.*.test_threads can take 10 minutes #113205

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@encukou
pythongh-113205: test_multiprocessing.test_terminate: Test the API wo…
f34cfdb
…rks on threadpools

Threads can't be forced to terminate (without potentially corrupting too much
state), so the  expected behaviour of `ThreadPool.terminate` is to wait for
the currently executing tasks to finish.

The entire test was skipped in pythonGH-110848 (0e9c364).
Instead of skipping it entirely, we should ensure the API eventually succeeds:
use a shorter timeout.

For the record: on my machine, when the test is un-skipped, the task manages to
start in about 1.5% cases.
@encukou encukou added skip news needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Jan 17, 2024
@bedevere-app bedevere-app Bot added the awaiting core review label Jan 17, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Jan 17, 2024
Occasionally multiprocessing.test.*.test_threads can take 10 minutes #113205
Closed
@encukou encukou mentioned this pull request Jan 17, 2024
GH-110829: Ensure Thread.join() joins the OS thread #110848
Merged
pitrou
pitrou approved these changes Jan 17, 2024
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Jan 17, 2024
gpshead
gpshead approved these changes Jan 18, 2024
View reviewed changes

@gpshead gpshead left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh nice, yay!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@gpshead
gpshead merged commit c1db960 into python:main Jan 18, 2024
@miss-islington-app

miss-islington-app Bot commented Jan 18, 2024

Copy link
Copy Markdown

Thanks @encukou for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the awaiting merge label Jan 18, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 18, 2024
@encukou @miss-islington
pythongh-113205: test_multiprocessing.test_terminate: Test the API on…
09cd0dc
… threadpools (pythonGH-114186)

pythongh-113205: test_multiprocessing.test_terminate: Test the API works on threadpools

Threads can't be forced to terminate (without potentially corrupting too much
state), so the  expected behaviour of `ThreadPool.terminate` is to wait for
the currently executing tasks to finish.

The entire test was skipped in pythonGH-110848 (0e9c364).
Instead of skipping it entirely, we should ensure the API eventually succeeds:
use a shorter timeout.

For the record: on my machine, when the test is un-skipped, the task manages to
start in about 1.5% cases.
(cherry picked from commit c1db960)

Co-authored-by: Petr Viktorin <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 18, 2024
@encukou @miss-islington
pythongh-113205: test_multiprocessing.test_terminate: Test the API on…
db5921b
… threadpools (pythonGH-114186)

pythongh-113205: test_multiprocessing.test_terminate: Test the API works on threadpools

Threads can't be forced to terminate (without potentially corrupting too much
state), so the  expected behaviour of `ThreadPool.terminate` is to wait for
the currently executing tasks to finish.

The entire test was skipped in pythonGH-110848 (0e9c364).
Instead of skipping it entirely, we should ensure the API eventually succeeds:
use a shorter timeout.

For the record: on my machine, when the test is un-skipped, the task manages to
start in about 1.5% cases.
(cherry picked from commit c1db960)

Co-authored-by: Petr Viktorin <[email protected]>
@bedevere-app

bedevere-app Bot commented Jan 18, 2024

Copy link
Copy Markdown

GH-114222 is a backport of this pull request to the 3.12 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label Jan 18, 2024
@bedevere-app

bedevere-app Bot commented Jan 18, 2024

Copy link
Copy Markdown

GH-114223 is a backport of this pull request to the 3.11 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the needs backport to 3.11 only security fixes label Jan 18, 2024
@encukou
encukou deleted the threadpool-terminate-test branch January 18, 2024 07:22
encukou added a commit that referenced this pull request Jan 18, 2024
@miss-islington @encukou
[3.12] gh-113205: test_multiprocessing.test_terminate: Shorter sleep …
c2a2126
…for threadpools (GH-114186) (GH-114222)

Threads can't be forced to terminate (without potentially corrupting too much
state), so the  expected behaviour of `ThreadPool.terminate` is to wait for
the currently executing tasks to finish.

Use shorter sleep time for threadpools, so if a task manages to start, the test
doesn't block for long.

(cherry picked from commit c1db960)

Co-authored-by: Petr Viktorin <[email protected]>
encukou added a commit that referenced this pull request Jan 18, 2024
@miss-islington @encukou
[3.11] gh-113205: test_multiprocessing.test_terminate: Shorter sleep …
569182c
…for threadpools (GH-114186) (GH-114223)

Threads can't be forced to terminate (without potentially corrupting too much
state), so the  expected behaviour of `ThreadPool.terminate` is to wait for
the currently executing tasks to finish.

Use shorter sleep time for threadpools, so if a task manages to start, the test
doesn't block for long.

(cherry picked from commit c1db960)

Co-authored-by: Petr Viktorin <[email protected]>
kulikjak pushed a commit to kulikjak/cpython that referenced this pull request Jan 22, 2024
@encukou @kulikjak
pythongh-113205: test_multiprocessing.test_terminate: Test the API on…
d03347f
… threadpools (python#114186)

pythongh-113205: test_multiprocessing.test_terminate: Test the API works on threadpools

Threads can't be forced to terminate (without potentially corrupting too much
state), so the  expected behaviour of `ThreadPool.terminate` is to wait for
the currently executing tasks to finish.

The entire test was skipped in pythonGH-110848 (0e9c364).
Instead of skipping it entirely, we should ensure the API eventually succeeds:
use a shorter timeout.

For the record: on my machine, when the test is un-skipped, the task manages to
start in about 1.5% cases.
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
@encukou @aisk
pythongh-113205: test_multiprocessing.test_terminate: Test the API on…
2d366e1
… threadpools (python#114186)

pythongh-113205: test_multiprocessing.test_terminate: Test the API works on threadpools

Threads can't be forced to terminate (without potentially corrupting too much
state), so the  expected behaviour of `ThreadPool.terminate` is to wait for
the currently executing tasks to finish.

The entire test was skipped in pythonGH-110848 (0e9c364).
Instead of skipping it entirely, we should ensure the API eventually succeeds:
use a shorter timeout.

For the record: on my machine, when the test is un-skipped, the task manages to
start in about 1.5% cases.
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@encukou @Glyphack
pythongh-113205: test_multiprocessing.test_terminate: Test the API on…
ddf3788
… threadpools (python#114186)

pythongh-113205: test_multiprocessing.test_terminate: Test the API works on threadpools

Threads can't be forced to terminate (without potentially corrupting too much
state), so the  expected behaviour of `ThreadPool.terminate` is to wait for
the currently executing tasks to finish.

The entire test was skipped in pythonGH-110848 (0e9c364).
Instead of skipping it entirely, we should ensure the API eventually succeeds:
use a shorter timeout.

For the record: on my machine, when the test is un-skipped, the task manages to
start in about 1.5% cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gpshead gpshead gpshead approved these changes
@pitrou pitrou pitrou approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@encukou @gpshead @pitrou
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.