Issue29343
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.
Created on 2017-01-22 12:11 by christian.heimes, last changed 2022-04-11 14:58 by admin.
Messages (6) | |||
---|---|---|---|
msg286004 - (view) | Author: Christian Heimes (christian.heimes) * | 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) * | 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) * | 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) * | 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) * | 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. |
|||
msg404589 - (view) | Author: Christian Heimes (christian.heimes) * | Date: 2021-10-21 12:04 | |
Yury, I don't remember how I found the issue. It's still an issue in 3.9 and newer. |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022-04-11 14:58:42 | admin | set | github: 73529 |
2021-10-21 12:04:09 | christian.heimes | set | messages:
+ msg404589 versions: + Python 3.9, Python 3.10, Python 3.11, - Python 3.6, Python 3.7 |
2017-01-24 04:45:29 | yselivanov | set | nosy:
+ yselivanov messages: + msg286132 |
2017-01-23 22:03:48 | neologix | set | messages: + msg286121 |
2017-01-22 13:47:41 | martin.panter | set | nosy:
+ martin.panter messages: + msg286010 |
2017-01-22 13:23:20 | pitrou | set | nosy:
+ neologix, pitrou messages: + msg286009 |
2017-01-22 12:11:57 | christian.heimes | create |