This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Misleading documentation for socket.fromfd()
Type: Stage:
Components: Library (Lib) Versions:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: georg.brandl, mikeh-id, mlrsmith
Priority: normal Keywords:

Created on 2006-03-29 09:41 by mlrsmith, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Messages (6)
msg27950 - (view) Author: Michael Smith (mlrsmith) Date: 2006-03-29 09:41
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 */
msg27951 - (view) Author: Mike Howard (mikeh-id) Date: 2006-03-31 16:52
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'.

msg27952 - (view) Author: Michael Smith (mlrsmith) Date: 2006-03-31 17:12
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.

msg27953 - (view) Author: Mike Howard (mikeh-id) Date: 2006-03-31 17:49
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'.

msg27954 - (view) Author: Mike Howard (mikeh-id) Date: 2006-03-31 18:00
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.
msg27955 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2006-04-01 07:33
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.
History
Date User Action Args
2022-04-11 14:56:16adminsetgithub: 43110
2006-03-29 09:41:19mlrsmithcreate