◐ Shell
reader mode source ↗
Skip to content

bpo-31233: socketserver.ThreadingMixIn.server_close()#3523

Merged
vstinner merged 4 commits into
python:masterfrom
vstinner:socketserver
Sep 13, 2017
Merged

bpo-31233: socketserver.ThreadingMixIn.server_close()#3523
vstinner merged 4 commits into
python:masterfrom
vstinner:socketserver

Conversation

@vstinner

@vstinner vstinner commented Sep 13, 2017

Copy link
Copy Markdown
Member

socketserver.ThreadingMixIn now keeps a list of non-daemonic threads
to wait until all these threads complete in server_close().

https://bugs.python.org/issue31233

socketserver.ThreadingMixIn now keeps a list of non-daemonic threads
to wait until all these threads complete in server_close().
@vstinner vstinner changed the title bpo-31233: socketserver.ThreadingMixIn.server_close() Sep 13, 2017
@vstinner

Copy link
Copy Markdown
Member Author

TODO: re-enable skipped tests in test_logging

-@unittest.skipIf(True, "FIXME: bpo-30830")

Problem: test.test_logging.SocketHandlerTest.test_output() hangs by doing that.

@vstinner

Copy link
Copy Markdown
Member Author

Should server_close() calls self.close_request(request) on thread requests?

Fix SocketHandlerTest.tearDown(): close the socket handler before
stopping the server, so the server can join threads.
@vstinner vstinner changed the title [WIP] bpo-31233: socketserver.ThreadingMixIn.server_close() Sep 13, 2017
@vstinner

Copy link
Copy Markdown
Member Author

This change is a temporary solution until we agree how to handle these threads and child processes in socketserver.

@vstinner

Copy link
Copy Markdown
Member Author

This PR now requires PR #3524 to prevent warnings re-introduced in test_logging.

@vstinner vstinner merged commit b8f4163 into python:master Sep 13, 2017
@vstinner vstinner deleted the socketserver branch September 13, 2017 08:47
@Mariatta Mariatta removed the label Oct 8, 2017
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.

4 participants