homepage

This issue tracker has been migrated to GitHub , and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: socketserver.ThreadingMixIn leaks running threads after server_close()
Type: resource usage Stage: resolved
Components: Library (Lib) Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: martin.panter, ned.deily, pitrou, vstinner
Priority: Keywords: patch

Created on 2017年08月18日 16:24 by vstinner, last changed 2022年04月11日 14:58 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 3523 merged vstinner, 2017年09月13日 00:16
PR 3524 merged vstinner, 2017年09月13日 01:12
PR 5417 merged vstinner, 2018年01月29日 08:36
Repositories containing patches
https://github.com/python/cpython/commit/96a6cbc5b0aee267f47e5efb50fba183b52cd8c6
Messages (19)
msg300514 - (view) Author: STINNER Victor (vstinner) * (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 (vstinner) * (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 (vstinner) * (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 (vstinner) * (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 (vstinner) * (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 (vstinner) * (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 (vstinner) * (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 (vstinner) * (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 (vstinner) * (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
msg304619 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017年10月19日 13:32
The current code leaks memory since it never clears threads which completed. We need a cleanup function similar to ForkingMixIn.collect_children() which is called by handle_timeout() and service_actions().
We can check if a thread is alive: thread.is_alive(), to decide to remove it or not.
Moreover, maybe we should keep a list of weak references?
msg307048 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017年11月27日 11:04
Klon666 added https://github.com/python/cpython/commit/96a6cbc5b0aee267f47e5efb50fba183b52cd8c6 repository. But this URL is unrelated. It looks like a bot spamming the bug tracker and I don't see how to remove the URL...
msg311016 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018年01月28日 22:20
What's the status of this? Feature freeze is here.
msg311021 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018年01月28日 22:32
> What's the status of this?
The status is bad. The commit b8f4163da30e16c7cd58fe04f4b17e38d53cd57e is backward incompatible, it's not documented in the "Porting the Python 3.7" section of What's New in Python 3.7, and it's not possible to opt-in for the previous behaviour.
> Feature freeze is here.
IMHO it's ok to post-pone this issue to beta2.
IMHO it's better to wait until all threads complete on server_close(), and it's possible to workaround the issue using daemon_threads = True which can be seen as the option to opt-in for the previous behaviour.
msg311025 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018年01月28日 22:49
If we're going to leave this open until 3.7.0b2, can you at least add a brief warning in the "Porting section" for b1 that the issue is unresolved?
msg311092 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018年01月29日 11:10
New changeset db8189bb8db609ca3993dec31fb95fdbe134469a by Victor Stinner in branch 'master':
bpo-31233, bpo-31151: Document socketserver changes (#5417)
https://github.com/python/cpython/commit/db8189bb8db609ca3993dec31fb95fdbe134469a
msg316835 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018年05月16日 16:28
I created bpo-33540: "socketserver: Add an opt-in option to get Python 3.6 behaviour on server_close()". I set the release blocker priority on this new issue.
msg318115 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018年05月29日 22:00
The bug has been fixed in Python 3.7.
msg345795 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2019年06月17日 01:28
FYI the change here to remember all the thread objects ever created looks to be the cause of the memory leak reported in Issue 37193 
History
Date User Action Args
2022年04月11日 14:58:50adminsetgithub: 75416
2019年06月17日 01:28:15martin.pantersetnosy: + martin.panter
messages: + msg345795
2018年05月29日 22:00:51vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg318115

stage: patch review -> resolved
2018年05月16日 16:28:14vstinnersetpriority: deferred blocker ->

messages: + msg316835
2018年01月29日 11:10:24vstinnersetmessages: + msg311092
2018年01月29日 08:36:21vstinnersetpull_requests: + pull_request5251
2018年01月28日 22:49:01ned.deilysetmessages: + msg311025
2018年01月28日 22:32:55vstinnersetmessages: + msg311021
2018年01月28日 22:20:30ned.deilysetmessages: + msg311016
2017年11月27日 11:04:39vstinnersetmessages: + msg307048
2017年11月25日 14:30:40berker.peksagsetnosy: - Tu madre
type: security -> resource usage
components: - Build
2017年11月25日 13:56:24Tu madresetnosy: + Tu madre
type: resource usage -> security
components: + Build
2017年11月25日 13:48:20Tu madresethgrepos: + hgrepo376
2017年10月19日 13:32:28vstinnersetmessages: + msg304619
2017年09月18日 19:39:47ned.deilysetpriority: release blocker -> deferred blocker
2017年09月13日 09:36:22vstinnerunlinkissue30830 dependencies
2017年09月13日 08:47:24vstinnersetmessages: + msg302037
2017年09月13日 08:44:14vstinnersetmessages: + msg302034
2017年09月13日 01:17:24vstinnersetmessages: + msg302024
2017年09月13日 01:12:28vstinnersetpull_requests: + pull_request3524
2017年09月13日 00:42:51vstinnersetpriority: normal -> release blocker
nosy: + ned.deily
messages: + msg302014

2017年09月13日 00:16:16vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request3521
2017年09月13日 00:05:24vstinnersetmessages: + msg302010
2017年09月13日 00:04:51vstinnersetmessages: + msg302009
2017年08月18日 21:47:56vstinnersetmessages: + msg300536
2017年08月18日 21:35:30vstinnersetmessages: + msg300533
2017年08月18日 17:29:29vstinnerlinkissue30830 dependencies
2017年08月18日 17:01:04pitrousetnosy: + pitrou
messages: + msg300518
2017年08月18日 16:24:59vstinnercreate

AltStyle によって変換されたページ (->オリジナル) /