Misleading documentation for socket.fromfd()

[mlrsmith]

BPO	1460564
Nosy	@birkenfeld

^{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 2006-04-01.07:33:46.000>
created_at = <Date 2006-03-29.09:41:19.000>
labels = ['library']
title = 'Misleading documentation for socket.fromfd()'
updated_at = <Date 2006-04-01.07:33:46.000>
user = 'https://bugs.python.org/mlrsmith'

bugs.python.org fields:

activity = <Date 2006-04-01.07:33:46.000>
actor = 'georg.brandl'
assignee = 'none'
closed = True
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2006-03-29.09:41:19.000>
creator = 'mlrsmith'
dependencies = []
files = []
hgrepos = []
issue_num = 1460564
keywords = []
message_count = 6.0
messages = ['27950', '27951', '27952', '27953', '27954', '27955']
nosy_count = 3.0
nosy_names = ['georg.brandl', 'mikeh-id', 'mlrsmith']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue1460564'
versions = []

[mlrsmith]

The socket.fromfd() method does not correctly document
what it does, in a way that is likely to cause users to
leak file descriptors.

Proposed fix:

--- Modules/socketmodule.c      2005-09-14
20:15:03.000000000 +0200
+++
/home/msmith/src/Python-2.4.2/Modules/socketmodule.c  
     2006-03-29 11:28:35.000000000 +0200
@@ -3077,7 +3077,8 @@
 PyDoc_STRVAR(fromfd_doc,
 "fromfd(fd, family, type[, proto]) -> socket object\n\
 \n\
-Create a socket object from the given file descriptor.\n\
+Duplicate the given file descriptor, and create a
socket\n\
+object from the duplicate.\r\
 The remaining arguments are the same as for socket().");

#endif /* NO_DUP */

[mikeh-id]

Logged In: YES
user_id=1195975

I don't believe the comment is incorrect, but I think the
code should be changed to reflect the comment.

socket.fromfd() is designed to be used when stdin/out/err
are inhereted from an invoking process - most probably
inetd. In this case, we get a file descriptor for an entity
which is really a socket and we need a socket in order to
perform i/o properly. Consequently, I think it is an error
to dup() the fd within 'fromfd'.

[mlrsmith]

Logged In: YES
user_id=1488997

It is not an error to dup() the fd here, but it IS behaviour
that the process calling socket.fromfd() must know about.

Also, changing this behaviour will probably break most users
of this API, since I don't see any way to use it correctly
without knowing that it does, in fact, dup() the FD (my code
reviously leaked file descriptors because I didn't know
this). Breaking existing applications is bad, hence my
suggestion to augment the documentation.

FWIW: my app receives file descriptors - which are sockets -
over a unix socket using sendmsg/recvmsg, for which I have
an extension written in C. Using socket.fromfd() on this FD
works perfectly, but I need to then os.close(fd) the
original FD to avoid leaking it.

[mikeh-id]

Logged In: YES
user_id=1195975

I don't believe the comment is incorrect, but I think the
code should be changed to reflect the comment.

socket.fromfd() is designed to be used when stdin/out/err
are inhereted from an invoking process - most probably
inetd. In this case, we get a file descriptor for an entity
which is really a socket and we need a socket in order to
perform i/o properly. Consequently, I think it is an error
to dup() the fd within 'fromfd'.

[mikeh-id]

Logged In: YES
user_id=1195975

I still believe that fromfd() is in error in dup'ing the
supplied file descriptor. This is because fromfd() is
designed only for limited use and because duplication can be
handled in other ways. It is unlikely anyone would
intentitionally close both the file descriptor and the socket.

The Library Documentation for includes the comment "This
function is rarely needed, but can be used to get or set
socket options on a socket passed to a program as standard
input or output (such as a server started by the Unix inet
daemon). The socket is assumed to be in blocking mode.
Availability: Unix."

I can't conceive of any case where it would be necessary for
this function to dup() the file descriptor which couldn't be
handled using the socket object dup() method. Using the
socket object method would be more transparent than hiding
the dup() within fromfd() and makes the documentation
consistent.

Regarding breaking existing code:

• I just tested multiple calls of sys.stdin/stdout.close()
  and they run w/o error, so closing the socket derived from
  either will not cause problems if someone closes the
  underlying file descriptor.

[birkenfeld]

Logged In: YES
user_id=849994

I'm not going to remove the dup() since it may be relied
upon in existing code.

I've corrected the docs in rev. 43523, 43524.