Skip to content

[3.7] bpo-34130: Fix 2 race conditions in test_signal #8329

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

Merged
merged 2 commits into from
Jul 18, 2018
Merged

[3.7] bpo-34130: Fix 2 race conditions in test_signal #8329

merged 2 commits into from
Jul 18, 2018

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Jul 18, 2018

vstinner added 2 commits July 18, 2018 18:30
test_signal.test_socket(): On Windows, sometimes even if the C signal handler
succeed to write the signal number into the write end of the socketpair, the
test fails with a BlockingIOError on the non-blocking read.recv(1) because the
read end of the socketpair didn't receive the byte yet.

Fix the race condition on Windows by setting the read end as blocking.

(cherry picked from commit 99bb6df)
On Windows, sometimes test_signal.test_warn_on_full_buffer() fails to
fill the socketpair buffer. In that case, the C signal handler
succeed to write into the socket, it doesn't log the expected send
error, and so the test fail.

On Windows, the test now uses a timeout of 50 ms to fill the
socketpair buffer to fix this race condition.

Other changes:

* Begin with large chunk size to fill the buffer to speed up the
  test.
* Add error messages to assertion errors to more easily identify
  which assertion failed.
* Don't set the read end of the socketpair as non-blocking.

(cherry picked from commit 686b4b5)
@bedevere-bot bedevere-bot added tests Tests in the Lib/test dir awaiting merge labels Jul 18, 2018
@vstinner vstinner merged commit 296e572 into python:3.7 Jul 18, 2018
@vstinner vstinner deleted the test_signal37 branch July 18, 2018 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants