classification
Title: Uses of SocketServer.BaseServer.shutdown have a race
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.1, Python 3.2, Python 2.7, Python 2.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: jyasskin Nosy List: giampaolo.rodola, jyasskin, pitrou, zanella
Priority: normal Keywords: patch

Created on 2008-03-16 15:15 by jyasskin, last changed 2010-04-25 22:28 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
race_free_shutdown.patch jyasskin, 2008-03-16 15:15
shutdown.patch pitrou, 2010-04-17 21:39
Messages (6)
msg63579 - (view) Author: Jeffrey Yasskin (jyasskin) * (Python committer) Date: 2008-03-16 15:15
With the code as it stands, calls to shutdown that happen before
serve_forever enters its loop will deadlock, and there's no simple way
for the user to avoid this. The attached patch prevents the deadlock and
allows multiple serve_forever..shutdown cycles, but it's pretty
complicated. I could make it a lot simpler by making shutdown permanent:
any later serve_forever calls would return immediately.

A third choice would be to add a .serve_in_thread function that returns
a token that can be used to shut down exactly that loop, instead of
putting .shutdown() on the server. Any opinions?
msg66364 - (view) Author: Rafael Zanella (zanella) Date: 2008-05-07 18:16
>With the code as it stands, calls to shutdown that happen before
>serve_forever enters its loop will deadlock, and there's no simple way
>for the user to avoid this. The attached patch prevents the deadlock and
>allows multiple serve_forever..shutdown cycles, but it's pretty
>complicated. I could make it a lot simpler by making shutdown permanent:
>any later serve_forever calls would return immediately.

Never thought of using the SocketServer taht way, wouldn't the person
doing this bunch of shutdown()s and serve_forever()s be better off using
handle_request() on a loop instead ?

>A third choice would be to add a .serve_in_thread function that returns
>a token that can be used to shut down exactly that loop, instead of
>putting .shutdown() on the server. Any opinions?

I don't think I understand this part, what loop do you refer to ?
msg103431 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-04-17 20:51
After a bit of investigation, this issue seems to be exactly why test_httpservers sometimes hangs.
The patch looks complicated to me, though; I don't think we really have to support the multiple shutdowns case.
msg103433 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-04-17 21:39
Here is a simpler patch. It also fixes the wrong use of a lock instead of an event in test_httpservers. With this patch, test_httpservers runs forever without freezing.
msg103626 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-04-19 19:46
By the way, getting rid of poll_interval for a file descriptor is easy under Unix, but wouldn't work under Windows (where select() only takes sockets, not arbitrary file descriptors).
msg104173 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-04-25 22:28
Fixed with a test in r80484 (trunk), r80486 (2.6), r80487 (py3k), r80491 (3.1).
History
Date User Action Args
2010-04-25 22:28:59pitrousetstatus: open -> closed
messages: + msg104173

keywords: patch, patch
resolution: fixed
stage: resolved
2010-04-19 19:46:16pitrousetkeywords: patch, patch

messages: + msg103626
2010-04-17 21:39:04pitrousetkeywords: patch, patch
files: + shutdown.patch
messages: + msg103433
2010-04-17 20:51:23pitrousetpriority: normal
versions: + Python 3.1, Python 2.7, Python 3.2
nosy: + pitrou

messages: + msg103431

keywords: patch, patch
2008-06-04 02:13:29giampaolo.rodolasetnosy: + giampaolo.rodola
2008-05-07 18:16:15zanellasetnosy: + zanella
messages: + msg66364
2008-03-16 22:46:56jyasskinsetkeywords: patch, patch
assignee: jyasskin
2008-03-16 17:20:30jyasskinlinkissue1193577 superseder
2008-03-16 15:15:55jyasskincreate