gh-69282: test_httpservers hangs since Python 3.5 #9564
Conversation
|
@vadmium what do you think about this PR? |
|
@WildCard65, please address the comments that @vadmium left on the bug tracker for this. Thank you! |
Added 'self.con.close()' after 'self.con.getresponse()' to close the socket connection. Kept the 'Connection' header in the request to signal our intent to terminate the socket.
|
@csabella I made the requested changes. |
|
This is ready for review. |
|
This missed the boat for inclusion in Python 3.9 which accepts security fixes only as of today. |
|
This issue and PR are no longer needed in that test_httpservers runs fine since 3.9. If some coredev familiar enough with httpserver thinks that the changes are a good idea anyway, please merge. |
|
Do we close this or do we remove the pending label? |
There was a problem hiding this comment.
I think the current proposed changes should fix the firewall problem as I understand it. The changes are a bit clumsy, but overall beneficial.
I don’t see any reason why the problem would already be fixed. Maybe @terryjreedy it’s just that you originally had a firewall interfering, but it isn’t any more?
| self.send_response(HTTPStatus.OK) | ||
| self.send_header('Content-Length', 0) |
There was a problem hiding this comment.
(Nitpick) It’s not really documented, and seems to work regardless, but I assume the header field value should be a string '0' not an integer
| # To combat this, we send the test server the "Connection" header | ||
| # with "close" for the value forcing the server and client to | ||
| # terminate the socket allowing the test to resume. | ||
| self.con.request('GET', '/', headers={'Connection': 'close'}) |
There was a problem hiding this comment.
The explanation seems vague. I would explain it as:
“Connection: close” causes the server to set its “close_connection” flag. If the client did not also shut down the connection below, this flag would avoid a deadlock between the server waiting for a second request on the connection, and the main thread shutting the server down.
|
I have no idea why the test failed and then passed again on my machine. It works now as is on a debug no-gil build. I do know that code can be improved, and maybe made more stable, even if not failing on a particular machine at a particular time. Please judge on that basis. I also know that tests that pass on GH CI machines may fail on my machine, and vice versa, and need revision to run better on both and others. |
|
This PR is stale because it has been open for 30 days with no activity. |
Added the following headers to RequestHandlerLoggingTestCase.test_get HTTP client request: Connection (value; close), Content-Length (value: 0)
This is a remake of the PR due to screw up I made in the last one.
https://bugs.python.org/issue25095