socketserver.ThreadingMixIn leaks running threads after server_close()

[vstinner]

BPO	31233
Nosy	@pitrou, @vstinner, @ned-deily, @vadmium
PRs	bpo-31233: socketserver.ThreadingMixIn.server_close() #3523 bpo-30830: logging.config.listen() calls server_close() #3524 bpo-31233, bpo-31151: Document socketserver changes #5417

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2018-05-29.22:00:51.118>
created_at = <Date 2017-08-18.16:24:59.372>
labels = ['3.7', 'library', 'performance']
title = 'socketserver.ThreadingMixIn leaks running threads after server_close()'
updated_at = <Date 2019-06-17.01:28:15.814>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2019-06-17.01:28:15.814>
actor = 'martin.panter'
assignee = 'none'
closed = True
closed_date = <Date 2018-05-29.22:00:51.118>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2017-08-18.16:24:59.372>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = ['376']
issue_num = 31233
keywords = ['patch']
message_count = 19.0
messages = ['300514', '300518', '300533', '300536', '302009', '302010', '302014', '302024', '302034', '302037', '304619', '307048', '311016', '311021', '311025', '311092', '316835', '318115', '345795']
nosy_count = 4.0
nosy_names = ['pitrou', 'vstinner', 'ned.deily', 'martin.panter']
pr_nums = ['3523', '3524', '5417']
priority = None
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue31233'
versions = ['Python 3.7']

[vstinner]

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.

[pitrou]

The difference is that letting a thread run doesn't create a zombie thread, so I don't think the issue is really similar.

[vstinner]

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)

[vstinner]

New changeset 6966960 by Victor Stinner in branch 'master':
bpo-30830: test_logging uses threading_setup/cleanup (bpo-3137)
6966960

[vstinner]

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

[vstinner]

Multiple test_logging tests have been skipped until this issue is fixed: see bpo-30830 and commit 6966960.

[vstinner]

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.

[vstinner]

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?

[vstinner]

New changeset 97d7e65 by Victor Stinner in branch 'master':
bpo-30830: logging.config.listen() calls server_close() (bpo-3524)
97d7e65

[vstinner]

New changeset b8f4163 by Victor Stinner in branch 'master':
bpo-31233: socketserver.ThreadingMixIn.server_close() (bpo-3523)
b8f4163