classification
Title: socketserver.ThreadingMixIn leaks running threads after server_close()
Type: resource usage Stage: patch review
Components: Library (Lib) Versions: Python 3.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: haypo, ned.deily, pitrou
Priority: deferred blocker Keywords: patch

Created on 2017-08-18 16:24 by haypo, last changed 2017-09-18 19:39 by ned.deily.

Pull Requests
URL Status Linked Edit
PR 3523 merged haypo, 2017-09-13 00:16
PR 3524 merged haypo, 2017-09-13 01:12
Messages (10)
msg300514 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-08-18 16:24
test_logging has a race condition: sometimes, it leaks dangling threads on FreeBSD: bpo-30830.

I identified the root issue: socketserver.ThreadingMixIn spawns threads without waiting for their completion in server_close(). This issue is very similar to socketserver.ForkingMixIn which leaks child processes and so create zombie processes. See bpo-31151.
msg300518 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2017-08-18 17:01
The difference is that letting a thread run doesn't create a zombie thread, so I don't think the issue is really similar.
msg300533 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-08-18 21:35
Oh, test_socketserver just failed with ENV_CHANGED on AMD64 Windows7 SP1 3.x:

http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/865/steps/test/logs/stdio

test_TCPServer (test.test_socketserver.SocketServerTest) ... creating server
(...)
waiting for server
done

Warning -- threading_cleanup() failed to cleanup -1 threads after 5 sec (count: 0, dangling: 1)
msg300536 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-08-18 21:47
New changeset 6966960468327c958b03391f71f24986bd697307 by Victor Stinner in branch 'master':
bpo-30830: test_logging uses threading_setup/cleanup (#3137)
https://github.com/python/cpython/commit/6966960468327c958b03391f71f24986bd697307
msg302009 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-09-13 00:04
Another recent example while testing bpo-31234:

test_ThreadingUDPServer (test.test_socketserver.SocketServerTest) ...
(...)
done
Warning -- threading_cleanup() detected 1 leaked threads (count: 1, dangling: 2)
ok
msg302010 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-09-13 00:05
Multiple test_logging tests have been skipped until this issue is fixed: see bpo-30830 and commit 6966960468327c958b03391f71f24986bd697307.
msg302014 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-09-13 00:42
Attached PR 3523 is a temporary solution until we agree how to handle these threads and child processes in socketserver. I wrote the PR to fix "dangling threads" warnings to fix random buildbot failures.

The PR fixes warnings but also reenable test_logging tests which are currently skipped. These tests were skipped by bpo-30830 to prevent random buildbot failures.

See also the thread on the python-dev mailing list to discuss about bpo-31151 "socketserver.ForkingMixIn.server_close() leaks zombie processes":
https://mail.python.org/pipermail/python-dev/2017-August/148826.html

I tag this issue as release blocker as a remainder that we have to agree how to handle threads/processes before Python 3.7 feature freeze.
msg302024 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-09-13 01:17
Again, PR 3523 is a temporary fix to unblock the situation.

For a proper fix, I have a question: Should server_close() calls self.close_request(request) on thread requests?
msg302034 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-09-13 08:44
New changeset 97d7e65dfed1d42d40d9bc2f630af56240555f02 by Victor Stinner in branch 'master':
bpo-30830: logging.config.listen() calls server_close() (#3524)
https://github.com/python/cpython/commit/97d7e65dfed1d42d40d9bc2f630af56240555f02
msg302037 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-09-13 08:47
New changeset b8f4163da30e16c7cd58fe04f4b17e38d53cd57e by Victor Stinner in branch 'master':
bpo-31233: socketserver.ThreadingMixIn.server_close() (#3523)
https://github.com/python/cpython/commit/b8f4163da30e16c7cd58fe04f4b17e38d53cd57e
History
Date User Action Args
2017-09-18 19:39:47ned.deilysetpriority: release blocker -> deferred blocker
2017-09-13 09:36:22haypounlinkissue30830 dependencies
2017-09-13 08:47:24hayposetmessages: + msg302037
2017-09-13 08:44:14hayposetmessages: + msg302034
2017-09-13 01:17:24hayposetmessages: + msg302024
2017-09-13 01:12:28hayposetpull_requests: + pull_request3524
2017-09-13 00:42:51hayposetpriority: normal -> release blocker
nosy: + ned.deily
messages: + msg302014

2017-09-13 00:16:16hayposetkeywords: + patch
stage: patch review
pull_requests: + pull_request3521
2017-09-13 00:05:24hayposetmessages: + msg302010
2017-09-13 00:04:51hayposetmessages: + msg302009
2017-08-18 21:47:56hayposetmessages: + msg300536
2017-08-18 21:35:30hayposetmessages: + msg300533
2017-08-18 17:29:29haypolinkissue30830 dependencies
2017-08-18 17:01:04pitrousetnosy: + pitrou
messages: + msg300518
2017-08-18 16:24:59haypocreate