bpo-36123:race condition in test_socket

[nanjekyejoannah]

I have set timeout once the socket is created rather than setting timeout on the
connection.

cc @vstinner

https://bugs.python.org/issue36123

[vstinner]

In your NEWS entry, it's unclear why you need this change. I prefer to explicit say that you fix a race condition, and mention the test name from the start. Moreover, "_testWithTimeoutTriggeredSend" is the private method, whereas testWithTimeoutTriggeredSend() is the public test name.

Fix test_socket.testWithTimeoutTriggeredSend() race condition: timeout is now set once the socket is created in rather than on the connection.

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[eamanu]

LGTM. This have sense, but I don't know how to test it.

[vstinner]

I'm able to reproduce the race condition using:

diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py
index 7c5167d850..3f749b95f9 100644
--- a/Lib/test/test_socket.py
+++ b/Lib/test/test_socket.py
@@ -5577,7 +5577,7 @@ class SendfileUsingSendTest(ThreadedTCPSocketTest):
     FILESIZE = (10 * 1024 * 1024)  # 10 MiB
     BUFSIZE = 8192
     FILEDATA = b""
-    TIMEOUT = 2
+    TIMEOUT = 0.001
 
     @classmethod
     def setUpClass(cls):

I'm not sure that the PR is enough. I suggest to also use a longer timeout on accept(). @nanjekyejoannah: can you please also include the following change in your PR?

diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py
index 7c5167d850..e127838d96 100644
--- a/Lib/test/test_socket.py
+++ b/Lib/test/test_socket.py
@@ -5603,7 +5603,7 @@ class SendfileUsingSendTest(ThreadedTCPSocketTest):
         support.unlink(support.TESTFN)
 
     def accept_conn(self):
-        self.serv.settimeout(self.TIMEOUT)
+        self.serv.settimeout(MAIN_TIMEOUT)
         conn, addr = self.serv.accept()
         conn.settimeout(self.TIMEOUT)
         self.addCleanup(conn.close)

MAIN_TIMEOUT is 60 seconds, whereas TIMEOUT is "only" 2 seconds. The FreeBSD buildbot is really slow, so a longer timeout can help.

[vstinner]

LGTM.

[miss-islington]

Thanks @nanjekyejoannah for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

[bedevere-bot]

GH-12056 is a backport of this pull request to the 3.7 branch.