classification
Title: Cannot use backlog = 0 for sockets
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Daniel.Evers, exarkun, neologix, pitrou, python-dev, tarek
Priority: low Keywords: needs review, patch

Created on 2010-04-22 10:02 by Daniel.Evers, last changed 2011-05-10 17:22 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
socket_backlog0.patch Daniel.Evers, 2011-05-09 15:01 This patch allows (and forces) an minimum backlog of 0 for listen sockets. review
Messages (19)
msg103942 - (view) Author: Daniel Evers (Daniel.Evers) Date: 2010-04-22 10:02
I'm trying to rewrite a server application in python that accepts exactly 1 connection. I have a previous version written in C that can call listen() on a socket with a backlog of 0 connections, but this is not possible in python. In Modules/socketmodule.c (function socket_listen) the backlog is reset to at least "1".
msg103957 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2010-04-22 12:00
That's a non-portable user of listen()'s backlog, see http://www.opengroup.org/onlinepubs/009695399/functions/listen.html:
A backlog argument of 0 may allow the socket to accept connections, in which case the length of the listen queue may be set to an implementation-defined minimum value.

Furthermore, python's socket documentation makes it clear:
Enable a server to accept connections.  The backlog argument must be at least 1; it specifies the number of unaccepted connection that the system will allow before refusing new connections.

If you really want to only accept 1 connection (I'm talking of sending back a SYN-ACK, not an accept() call), you can just close the server socket as soon as you've done the accept, and re-open it when you're ready to process another request. Or you could accept new connections and close them immediately (but then you'd send back a SYN-ACK then a FIN).

So I'd say it's not a python bug.
msg103961 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2010-04-22 13:12
> Furthermore, python's socket documentation makes it clear:

Why does CPython go out of its way to make it impossible to pass 0 to the platform listen() API?  The part of the specification you quoted makes it very clear that 0 is a valid value for this argument.  And 0 seems to have a very different meaning than 1, but for some reason CPython thinks that 1 is a better thing for 0 to mean.

Perhaps there is a real reason someone thought to add this check in sock_listen, but if not, I think it's worth considering removing this check.

Unfortunately the commit message for the revision which added this check (r3880) is not informative:

sock_listen: ensure backlog argument is at least1

> If you really want to only accept 1 connection (I'm talking of sending back a SYN-ACK, not an accept() call), you can just close the server socket

This makes perfect sense, though, and is good advice in general.
msg103978 - (view) Author: Daniel Evers (Daniel.Evers) Date: 2010-04-22 20:01
Thanks for your help. I'll close the listen socket.

Nevertheless I'd like to understand the check in sock_listen. The commit message is not very helpful, I agree, that's why I wrote this issue. 0 is a valid value, and I'd suggest two ways to handle this situation:
1. Always set backlog at least to 1 (like now), but make this explicit in the documentation (docs.python.org says "should be at least 1" which is not the same as "will be at least 1")
2. Allow backlog = 0 and make a note in the docs that the exact behaviour will be implementation/OS-specific.

In case (1) the hint to close the listen socket should also be added to the documentation.
msg103986 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-04-22 23:16
The request to allow 0 looks reasonable to me. Now if someone wants to propose a patch (+ tests if there's any way of testing for this).
msg104072 - (view) Author: Daniel Evers (Daniel.Evers) Date: 2010-04-24 09:42
I attached a patch:
The backlog is set to at least 0 instead of 1. I also added a comment that a backlog < 0 can lead to problems and doesn't make sense anyway (so if there are systems that may crash with backlog < 0 this will be avoided).
msg104073 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2010-04-24 09:57
It looks like you forgot to update the function's documentation ;-)
msg104075 - (view) Author: Daniel Evers (Daniel.Evers) Date: 2010-04-24 11:54
Ah right ;)
Sorry, attached the path incl. the doc string.
msg104076 - (view) Author: Daniel Evers (Daniel.Evers) Date: 2010-04-24 11:58
A second patch for the documentation of socket.listen().
msg104137 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2010-04-25 01:16
It'd be nice to have a unit test that passes a small enough value to listen() to trigger the check.  Since there's no way to reliably determine what the system backlog really is, there's probably no reason to actually try to determine that the "right" value was passed to listen.  Just making sure the code actually goes through the success-case when this check is hit is probably enough.
msg134962 - (view) Author: Daniel Evers (Daniel.Evers) Date: 2011-05-02 12:12
To revive this issue, I tried to write a unit test to verify the behaviour. Onfurtunately, the test doesn't work and I don't understand why. I hope, someone here is more enlightend than me...
(files: server.py, client.py)
msg134963 - (view) Author: Daniel Evers (Daniel.Evers) Date: 2011-05-02 12:12
(client.py)
msg135070 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-05-03 21:00
> To revive this issue, I tried to write a unit test to verify the behaviour.
> Onfurtunately, the test doesn't work and I don't understand why. I hope,
> someone here is more enlightend than me...

The semantic of listen's backlog argument has always been unclear, so
different implementations apply some "fudge factor" to the value
passed, see for example
http://books.google.com/books?id=ptSC4LpwGA0C&lpg=PA108&ots=Kq9FQogkTr&dq=berkeley%20listen%20backlog%20ack&pg=PA108#v=onepage&q=berkeley
listen backlog ack&f=false

Most of the time, passing <backlog> means that the kernel will queue
at least <backlog> requests.
Under the kernel I'm using (2.6.32), the maximum number of queued
requests is actually backlog + 1, and it seems to be the same on
FreeBSD (I guess one of the reason is to cope with backlog == 0).
Note that under Linux, there's another setting coming into play,
net.ipv4.tcp_abort_on_overflow:
"""
If listening service is too slow to accept new connections, reset
them. Not enabled by default. It means that if overflow occurred due
to a burst, connection will recover.
Enable this option only if you are really sure that listening daemon
cannot be tuned to accept connections faster. Enabling this option can
harm clients of your server.
"""

The goal is to avoid dropping connection requests because of a temporary burst.
So if you change your script to make as many connections as possible
with the default value, you'll probably be able to go well above
listen's backlog.
To deactivate it, you can use:
# sysctl  net.ipv4.tcp_abort_on_overflow=1

(But it's not a good idea in a production setting).

As for the test, you should add it to Lib/test/test_socket.py (just
creating a socket, binding it and passing a backlog of 0 should be
more than enough).
msg135105 - (view) Author: Daniel Evers (Daniel.Evers) Date: 2011-05-04 09:40
Thanks for the tip. I added the unit test and uploaded my final patch (which includes all changes).
Is it ok to remove the files I uploaded previously?
msg135230 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-05-05 18:53
> Thanks for the tip. I added the unit test and uploaded my final patch
> (which includes all changes).

A couple comments (note that I'm not entitled to accept or commit a patch, so feel free to ignore them if I'm just being a pain):
- "the maximum value is system-dependent (usually 5)"
I would remove this "usually 5" part: since the kernel silently adjusts the value, it shouldn't be of interest to the programmer. Furthermore, this was probably true a decade ago, but nowadays it doesn't hold anymore, Linux for example allows a much higher limit
- port = support.bind_port(srv)
Since you don't care about which port you get bound to, you could discard the port value, or even call srv.bind((HOST, 0)) directly
- I'm not sure that creating a new SocketListenBacklog0Test TestCase just for this check is necessary. Since it's so short, you could just add a new test to e.g. GeneralModuleTests

> Is it ok to remove the files I uploaded previously? 

It's probably a good idea, so there won't be any confusion with "stale" patches and test scripts.
msg135594 - (view) Author: Daniel Evers (Daniel.Evers) Date: 2011-05-09 15:01
Thanks, I removed the old patches and changed the unit test according to your suggestion.
I kept the "usually 5" remark, because I'm not sure how reality really looks like. But feel free to suggest a patch ;)
msg135651 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-05-09 21:10
The patch looks fine to me: you just need to find someone interested
in reviewing and committing it (didn't find anyone listed as expert
for the socket module).
msg135721 - (view) Author: Roundup Robot (python-dev) Date: 2011-05-10 17:20
New changeset 48743ad2d2ef by Antoine Pitrou in branch '2.7':
Issue #8498: In socket.accept(), allow to specify 0 as a backlog value in
http://hg.python.org/cpython/rev/48743ad2d2ef

New changeset 713006ecee0c by Antoine Pitrou in branch '3.2':
Issue #8498: In socket.accept(), allow to specify 0 as a backlog value in
http://hg.python.org/cpython/rev/713006ecee0c

New changeset 5e1ed84883c5 by Antoine Pitrou in branch 'default':
Issue #8498: In socket.accept(), allow to specify 0 as a backlog value in
http://hg.python.org/cpython/rev/5e1ed84883c5
msg135722 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-05-10 17:22
Committed, thanks for the patch and review!
History
Date User Action Args
2011-05-10 17:22:03pitrousetstatus: open -> closed
versions: + Python 3.3, - Python 2.6, Python 3.1
messages: + msg135722

resolution: fixed
stage: resolved
2011-05-10 17:20:49python-devsetnosy: + python-dev
messages: + msg135721
2011-05-09 21:10:23neologixsetmessages: + msg135651
2011-05-09 15:01:02Daniel.Everssetfiles: + socket_backlog0.patch

messages: + msg135594
2011-05-09 14:57:24Daniel.Everssetfiles: - backlog0_complete.patch
2011-05-09 14:57:21Daniel.Everssetfiles: - client.py
2011-05-09 14:57:19Daniel.Everssetfiles: - server.py
2011-05-09 14:57:17Daniel.Everssetfiles: - socket_listen.patch
2011-05-09 14:57:14Daniel.Everssetfiles: - backlog0_incl_doc.diff
2011-05-09 14:57:12Daniel.Everssetfiles: - backlog0.diff
2011-05-05 18:53:48neologixsetkeywords: + needs review

messages: + msg135230
2011-05-04 09:40:16Daniel.Everssetfiles: + backlog0_complete.patch

messages: + msg135105
2011-05-03 21:00:23neologixsetmessages: + msg135070
2011-05-02 12:12:28Daniel.Everssetfiles: + client.py

messages: + msg134963
2011-05-02 12:12:13Daniel.Everssetfiles: + server.py

messages: + msg134962
2010-04-25 01:16:20exarkunsetmessages: + msg104137
2010-04-24 11:58:27Daniel.Everssetfiles: + socket_listen.patch

messages: + msg104076
2010-04-24 11:54:24Daniel.Everssetfiles: + backlog0_incl_doc.diff

messages: + msg104075
2010-04-24 09:57:20neologixsetmessages: + msg104073
2010-04-24 09:42:26Daniel.Everssetfiles: + backlog0.diff
keywords: + patch
messages: + msg104072
2010-04-22 23:16:37pitrousetpriority: low
versions: + Python 3.1, Python 2.7, Python 3.2
2010-04-22 23:16:20pitrousetnosy: + pitrou
messages: + msg103986
2010-04-22 20:02:01Daniel.Everssetmessages: + msg103978
2010-04-22 13:22:25tareksetassignee: tarek ->
components: + Library (Lib), - Distutils
2010-04-22 13:12:44exarkunsetmessages: + msg103961
2010-04-22 12:00:30neologixsetnosy: + tarek, neologix
messages: + msg103957

assignee: tarek
components: + Distutils, - Extension Modules
2010-04-22 10:33:18pitrousetnosy: + exarkun
2010-04-22 10:02:51Daniel.Everscreate