msg300514 - (view) |
Author: STINNER Victor (vstinner) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
Date: 2018-01-28 22:20 |
What's the status of this? Feature freeze is here.
|
msg311021 - (view) |
Author: STINNER Victor (vstinner) *  |
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) *  |
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) *  |
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) *  |
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) *  |
Date: 2018-05-29 22:00 |
The bug has been fixed in Python 3.7.
|
msg345795 - (view) |
Author: Martin Panter (martin.panter) *  |
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
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:50 | admin | set | github: 75416 |
2019-06-17 01:28:15 | martin.panter | set | nosy:
+ martin.panter messages:
+ msg345795
|
2018-05-29 22:00:51 | vstinner | set | status: open -> closed resolution: fixed messages:
+ msg318115
stage: patch review -> resolved |
2018-05-16 16:28:14 | vstinner | set | priority: deferred blocker ->
messages:
+ msg316835 |
2018-01-29 11:10:24 | vstinner | set | messages:
+ msg311092 |
2018-01-29 08:36:21 | vstinner | set | pull_requests:
+ pull_request5251 |
2018-01-28 22:49:01 | ned.deily | set | messages:
+ msg311025 |
2018-01-28 22:32:55 | vstinner | set | messages:
+ msg311021 |
2018-01-28 22:20:30 | ned.deily | set | messages:
+ msg311016 |
2017-11-27 11:04:39 | vstinner | set | messages:
+ msg307048 |
2017-11-25 14:30:40 | berker.peksag | set | nosy:
- Tu madre type: security -> resource usage components:
- Build
|
2017-11-25 13:56:24 | Tu madre | set | nosy:
+ Tu madre type: resource usage -> security components:
+ Build
|
2017-11-25 13:48:20 | Tu madre | set | hgrepos:
+ hgrepo376 |
2017-10-19 13:32:28 | vstinner | set | messages:
+ msg304619 |
2017-09-18 19:39:47 | ned.deily | set | priority: release blocker -> deferred blocker |
2017-09-13 09:36:22 | vstinner | unlink | issue30830 dependencies |
2017-09-13 08:47:24 | vstinner | set | messages:
+ msg302037 |
2017-09-13 08:44:14 | vstinner | set | messages:
+ msg302034 |
2017-09-13 01:17:24 | vstinner | set | messages:
+ msg302024 |
2017-09-13 01:12:28 | vstinner | set | pull_requests:
+ pull_request3524 |
2017-09-13 00:42:51 | vstinner | set | priority: normal -> release blocker nosy:
+ ned.deily messages:
+ msg302014
|
2017-09-13 00:16:16 | vstinner | set | keywords:
+ patch stage: patch review pull_requests:
+ pull_request3521 |
2017-09-13 00:05:24 | vstinner | set | messages:
+ msg302010 |
2017-09-13 00:04:51 | vstinner | set | messages:
+ msg302009 |
2017-08-18 21:47:56 | vstinner | set | messages:
+ msg300536 |
2017-08-18 21:35:30 | vstinner | set | messages:
+ msg300533 |
2017-08-18 17:29:29 | vstinner | link | issue30830 dependencies |
2017-08-18 17:01:04 | pitrou | set | nosy:
+ pitrou messages:
+ msg300518
|
2017-08-18 16:24:59 | vstinner | create | |