classification
Title: sock.close() raises OSError EBADF when socket's fd is closed
Type: behavior Stage:
Components: Library (Lib) Versions: Python 3.7, Python 3.6
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: christian.heimes, martin.panter, neologix, pitrou, yselivanov
Priority: normal Keywords: 3.6regression

Created on 2017-01-22 12:11 by christian.heimes, last changed 2017-01-24 04:45 by yselivanov.

Messages (5)
msg286004 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2017-01-22 12:11
In Python 3.6 the behavior of socket's close method has changed. In Python 2.7, 3.5 and earlier, socket.close() ignored EBADF. Starting with Python 3.6 socket.close() raises an OSError exception when its internal fd has been closed.

Python 2.7.12 (default, Sep 29 2016, 12:52:02) 
[GCC 6.2.1 20160916 (Red Hat 6.2.1-2)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import os, socket
>>> sock = socket.socket()
>>> os.close(sock.fileno())
>>> sock.close()

Python 3.5.2 (default, Sep 14 2016, 11:28:32) 
[GCC 6.2.1 20160901 (Red Hat 6.2.1-1)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import socket
>>> sock = socket.socket()
>>> import os, socket
>>> os.close(sock.fileno())
>>> sock.close()

Python 3.6.0+ (3.6:ea0c488b9bac, Jan 14 2017, 14:08:17) 
[GCC 6.3.1 20161221 (Red Hat 6.3.1-1)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import os, socket
>>> sock = socket.socket()
>>> os.close(sock.fileno())
>>> sock.close()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/heimes/dev/python/python36/lib/python3.6/socket.py", line 417, in close
    self._real_close()
  File "/home/heimes/dev/python/python36/lib/python3.6/socket.py", line 411, in _real_close
    _ss.close(self)
OSError: [Errno 9] Bad file descriptor


Abstraction layers such as the socket class should ignore EBADF in close(). A debug message or a warning is ok, but an exception is wrong. Even the man page for close discourages it, http://man7.org/linux/man-pages/man2/close.2.html

       Note, however, that a failure return should be used only for
       diagnostic purposes (i.e., a warning to the application that there
       may still be I/O pending or there may have been failed I/O) or
       remedial purposes (e.g., writing the file once more or creating a
       backup).
msg286009 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2017-01-22 13:23
Closing a socket whose fd has already been closed before is a bug waiting to happen.  Indeed it feels harmless if you just get a EBADF, but if the given fd gets reused in the meantime for another file or socket, your close() method is going to close a resource which doesn't belong to the socket (and then the fd can be reused for yet another resource without its owner knowing, and confusion/hilarity ensues).
msg286010 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2017-01-22 13:47
I added this behaviour in 3.6 on purpose via Issue 26685.

The change also impacted Yury (see the original bug thread), but if I understood correctly, he eventually decided that it highlighted a problem in asyncio or his code (though the resulting asyncio pull request seems to have stalled). Before he came to that decision, I did suggest temporarily using DeprecationWarning instead of an exception: <https://bugs.python.org/issue26685#msg278949>.

IMO passing a freed file descriptor to close() is asking for trouble. The descriptor could be recycled, by another thread, or even internally by a function call in the same thread. Another problem is if you don’t end up calling socket.close(), the garbage collector may arbitrarily close an FD in use in the future.

Your example code was not realistic, but I would say the solution is either call socket.detach() rather than socket.fileno(), or don’t call os.close() and just call socket.close().

I think that Linux man page is talking more about asynchronous errors from a previous write call. Even if we tolerated other errors from close(), I would still like to treat EBADF as a real error.
msg286121 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2017-01-23 22:03
FWIW I agree with Antoine and Martin: ignoring EBADF is a bad idea,
quite dangerous.
The man page probably says this to highlight that users shouldn't
*retry* close():
"""
       Retrying the close() after a failure return is the wrong thing to do,
       since this may cause a reused file descriptor from another thread to
       be closed.  This can occur because the Linux kernel always releases
       the file descriptor early in the close operation, freeing it for
       reuse; the steps that may return an error, such as flushing data to
       the filesystem or device, occur only later in the close operation.

"""

(Including on EINTR).

In short, I think that the change is a good idea: Socket.close() is
guaranteed to be idempotent, so this error can only happen in case of
invalid usage.
msg286132 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2017-01-24 04:45
Christian, how did you find this (i.e. what module/package is broken because of this)?  I'm still not entirely sure that raising EBADF in socket.close() is a good thing.
History
Date User Action Args
2017-01-24 04:45:29yselivanovsetnosy: + yselivanov
messages: + msg286132
2017-01-23 22:03:48neologixsetmessages: + msg286121
2017-01-22 13:47:41martin.pantersetnosy: + martin.panter
messages: + msg286010
2017-01-22 13:23:20pitrousetnosy: + neologix, pitrou
messages: + msg286009
2017-01-22 12:11:57christian.heimescreate