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

Improve stability of tests_subprocess #2205

Merged
merged 1 commit into from
Jan 8, 2022
Merged

Improve stability of tests_subprocess #2205

merged 1 commit into from
Jan 8, 2022

Conversation

tjstum
Copy link
Member

@tjstum tjstum commented Dec 24, 2021

On macOS, there appears to be some sort of race where sending SIGTERM
to a process "too early" in its lifetime causes the exit status to
appear as if SIGKILL was sent.
I can reproduce this with the stdlib subprocess. Additionally, this
problem doesn't occur if there is a tiny sleep between spawning a
child subprocess and terminating it.

My proposal is to use /bin/sleep on posix (which reliably does not
have this issue on macOS) and continue using the Python form elsewhere.

Related to #851. Originally submitted as #2190 (I was a bit too hasty in cleaning up my old branches)

@tjstum tjstum requested a review from pquentin December 24, 2021 00:20
@codecov
Copy link

codecov bot commented Dec 24, 2021

Codecov Report

Merging #2205 (6a19cab) into master (7b0001d) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 6a19cab differs from pull request most recent head d38a406. Consider uploading reports for the commit d38a406 to get more accurate results

@@            Coverage Diff             @@
##           master    #2205      +/-   ##
==========================================
- Coverage   99.51%   99.51%   -0.01%     
==========================================
  Files         114      114              
  Lines       14778    14761      -17     
  Branches     2344     2346       +2     
==========================================
- Hits        14706    14689      -17     
  Misses         47       47              
  Partials       25       25              
Impacted Files Coverage Δ
trio/tests/test_subprocess.py 100.00% <100.00%> (ø)
trio/_subprocess.py 96.86% <0.00%> (-0.03%) ⬇️
trio/_unix_pipes.py 100.00% <0.00%> (ø)
trio/_windows_pipes.py 100.00% <0.00%> (ø)
trio/_subprocess_platform/__init__.py 100.00% <0.00%> (ø)

@tjstum tjstum closed this Jan 7, 2022
@tjstum tjstum reopened this Jan 7, 2022
On macOS, there appears to be some sort of race where sending SIGTERM
to a process "too early" in its lifetime causes the exit status to
appear as if SIGKILL was sent.
I can reproduce this with the stdlib subprocess. Additionally, this
problem doesn't occur if there is a tiny sleep between spawning a
child subprocess and terminating it.

My proposal is to use `/bin/sleep` on posix (which reliably does not
have this issue on macOS) and continue using the Python form elsewhere.
@oremanj oremanj merged commit 147a545 into python-trio:master Jan 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants