<div dir="ltr"><div style="color:rgb(33,33,33);font-size:13px">I agree that blocking shutdown by default isn't a good idea. A child will eventually get indefinitely stuck on a nonresponsive connection and hang the whole server. This behavior change is surprising and should be reverted in master, and definitely not backported.</div><div style="color:rgb(33,33,33);font-size:13px"><br></div><div style="color:rgb(33,33,33);font-size:13px">As for block-timeout or block-timeout-kill, waiting more than zero seconds in server_close() should be optional, because you're right that the best timeout is circumstantial. Since ThreadingMixIn also leaks threads, server_close() could grow a timeout flag (following the socket module timeout convention) and maybe a terminate boolean. ThreadingMixIn could then also be fixed. I'm not sure how useful that is though, since I'd bet almost all users of socketserver exit the process shortly after server_close(). Plus it can't be backported to the feature-freeze branches.</div><div style="color:rgb(33,33,33);font-size:13px"><br></div><div style="color:rgb(33,33,33);font-size:13px">It seems like this is getting complicated enough that putting the fix in test_socketserver.py is probably best. Another solution is to add a secret terminating-close flag to ForkingMixIn just for the tests. Is that good practice in the stdlib?</div><br><div class="gmail_quote"><div dir="ltr">On Fri, Aug 11, 2017 at 6:46 AM Victor Stinner <<a href="mailto:victor.stinner@gmail.com">victor.stinner@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
I'm working on reducing the failure rate of Python CIs (Travis CI,<br>
AppVeyor, buildbots). For that, I'm trying to reduce test side effects<br>
using "environment altered" warnings. This week, I worked on<br>
support.reap_children() which detects leaked child processes (usually<br>
created with os.fork()).<br>
<br>
I found a bug in the socketserver module: it waits for child processes<br>
completion, but only in non-blocking mode. If a child process takes<br>
too long, the server will never reads its exit status and so the<br>
server leaks "zombie processes". Leaking processes can increase the<br>
memory usage, spawning new processes can fail, etc.<br>
<br>
=> <a href="http://bugs.python.org/issue31151" rel="noreferrer" target="_blank">http://bugs.python.org/issue31151</a><br>
<br>
I changed the code to call waitpid() in blocking mode on each child<br>
process on server_close(), to ensure that all children completed when<br>
on server close:<br>
<br>
<a href="https://github.com/python/cpython/commit/aa8ec34ad52bb3b274ce91169e1bc4a598655049" rel="noreferrer" target="_blank">https://github.com/python/cpython/commit/aa8ec34ad52bb3b274ce91169e1bc4a598655049</a><br>
<br>
After pushing my change, I'm not sure anymore if it's a good idea.<br>
There is a risk that server_close() blocks if a child is stuck on a<br>
socket recv() or send() for some reasons.<br>
<br>
Should we relax the code by waiting a few seconds (problem: hardcoded<br>
timeouts are always a bad idea), or *terminate* processes (SIGKILL on<br>
UNIX) if they don't complete fast enough?<br>
<br>
I don't know which applications use socketserver. How I can test if it<br>
breaks code in the wild?<br>
<br>
At least, I didn't notice any regression on Python CIs.<br>
<br>
Well, maybe the change is ok for the master branch. But I would like<br>
your opinion because now I would like to backport the fix to 2.7 and<br>
3.6 branches. It might break some applications.<br>
<br>
If we *cannot* backport such change to 2.7 and 3.6 because it changes<br>
the behaviour, I will fix the bug in test_socketserver.py instead.<br>
<br>
What do you think?<br>
<br>
Victor<br>
_______________________________________________<br>
Python-Dev mailing list<br>
<a href="mailto:Python-Dev@python.org" target="_blank">Python-Dev@python.org</a><br>
<a href="https://mail.python.org/mailman/listinfo/python-dev" rel="noreferrer" target="_blank">https://mail.python.org/mailman/listinfo/python-dev</a><br>
Unsubscribe: <a href="https://mail.python.org/mailman/options/python-dev/rmsr%40lab.net" rel="noreferrer" target="_blank">https://mail.python.org/mailman/options/python-dev/rmsr%40lab.net</a><br>
</blockquote></div></div>