[Python-checkins] cpython: Silently ignore unregistering closed files. Fixes issue 19876. With docs and
guido.van.rossum
python-checkins at python.org
Sun Dec 8 00:57:15 CET 2013
http://hg.python.org/cpython/rev/39e7995f9ad1
changeset: 87816:39e7995f9ad1
user: Guido van Rossum <guido at python.org>
date: Sat Dec 07 15:57:01 2013 -0800
summary:
Silently ignore unregistering closed files. Fixes issue 19876. With docs and slight test refactor.
files:
Doc/library/selectors.rst | 7 +-
Lib/selectors.py | 74 +++++++++++++++++--
Lib/test/test_selectors.py | 96 +++++++++++++++----------
3 files changed, 129 insertions(+), 48 deletions(-)
diff --git a/Doc/library/selectors.rst b/Doc/library/selectors.rst
--- a/Doc/library/selectors.rst
+++ b/Doc/library/selectors.rst
@@ -102,7 +102,8 @@
Register a file object for selection, monitoring it for I/O events.
- *fileobj* is the file object to monitor.
+ *fileobj* is the file object to monitor. It may either be an integer
+ file descriptor or an object with a ``fileno()`` method.
*events* is a bitwise mask of events to monitor.
*data* is an opaque object.
@@ -118,7 +119,9 @@
*fileobj* must be a file object previously registered.
This returns the associated :class:`SelectorKey` instance, or raises a
- :exc:`KeyError` if the file object is not registered.
+ :exc:`KeyError` if *fileobj* is not registered. It will raise
+ :exc:`ValueError` if *fileobj* is invalid (e.g. it has no ``fileno()``
+ method or its ``fileno()`` method has an invalid return value).
.. method:: modify(fileobj, events, data=None)
diff --git a/Lib/selectors.py b/Lib/selectors.py
--- a/Lib/selectors.py
+++ b/Lib/selectors.py
@@ -25,6 +25,9 @@
Returns:
corresponding file descriptor
+
+ Raises:
+ ValueError if the object is invalid
"""
if isinstance(fileobj, int):
fd = fileobj
@@ -55,7 +58,8 @@
def __getitem__(self, fileobj):
try:
- return self._selector._fd_to_key[_fileobj_to_fd(fileobj)]
+ fd = self._selector._fileobj_lookup(fileobj)
+ return self._selector._fd_to_key[fd]
except KeyError:
raise KeyError("{!r} is not registered".format(fileobj)) from None
@@ -89,6 +93,15 @@
Returns:
SelectorKey instance
+
+ Raises:
+ ValueError if events is invalid
+ KeyError if fileobj is already registered
+ OSError if fileobj is closed or otherwise is unacceptable to
+ the underlying system call (if a system call is made)
+
+ Note:
+ OSError may or may not be raised
"""
raise NotImplementedError
@@ -101,6 +114,13 @@
Returns:
SelectorKey instance
+
+ Raises:
+ KeyError if fileobj is not registered
+
+ Note:
+ If fileobj is registered but has since been closed this does
+ *not* raise OSError (even if the wrapped syscall does)
"""
raise NotImplementedError
@@ -114,6 +134,9 @@
Returns:
SelectorKey instance
+
+ Raises:
+ Anything that unregister() or register() raises
"""
self.unregister(fileobj)
return self.register(fileobj, events, data)
@@ -177,22 +200,41 @@
# read-only mapping returned by get_map()
self._map = _SelectorMapping(self)
+ def _fileobj_lookup(self, fileobj):
+ """Return a file descriptor from a file object.
+
+ This wraps _fileobj_to_fd() to do an exhaustive search in case
+ the object is invalid but we still have it in our map. This
+ is used by unregister() so we can unregister an object that
+ was previously registered even if it is closed. It is also
+ used by _SelectorMapping.
+ """
+ try:
+ return _fileobj_to_fd(fileobj)
+ except ValueError:
+ # Do an exhaustive search.
+ for key in self._fd_to_key.values():
+ if key.fileobj is fileobj:
+ return key.fd
+ # Raise ValueError after all.
+ raise
+
def register(self, fileobj, events, data=None):
if (not events) or (events & ~(EVENT_READ | EVENT_WRITE)):
raise ValueError("Invalid events: {!r}".format(events))
- key = SelectorKey(fileobj, _fileobj_to_fd(fileobj), events, data)
+ key = SelectorKey(fileobj, self._fileobj_lookup(fileobj), events, data)
if key.fd in self._fd_to_key:
- raise KeyError("{!r} (FD {}) is already "
- "registered".format(fileobj, key.fd))
+ raise KeyError("{!r} (FD {}) is already registered"
+ .format(fileobj, key.fd))
self._fd_to_key[key.fd] = key
return key
def unregister(self, fileobj):
try:
- key = self._fd_to_key.pop(_fileobj_to_fd(fileobj))
+ key = self._fd_to_key.pop(self._fileobj_lookup(fileobj))
except KeyError:
raise KeyError("{!r} is not registered".format(fileobj)) from None
return key
@@ -200,7 +242,7 @@
def modify(self, fileobj, events, data=None):
# TODO: Subclasses can probably optimize this even further.
try:
- key = self._fd_to_key[_fileobj_to_fd(fileobj)]
+ key = self._fd_to_key[self._fileobj_lookup(fileobj)]
except KeyError:
raise KeyError("{!r} is not registered".format(fileobj)) from None
if events != key.events:
@@ -352,7 +394,12 @@
def unregister(self, fileobj):
key = super().unregister(fileobj)
- self._epoll.unregister(key.fd)
+ try:
+ self._epoll.unregister(key.fd)
+ except OSError:
+ # This can happen if the FD was closed since it
+ # was registered.
+ pass
return key
def select(self, timeout=None):
@@ -409,11 +456,20 @@
if key.events & EVENT_READ:
kev = select.kevent(key.fd, select.KQ_FILTER_READ,
select.KQ_EV_DELETE)
- self._kqueue.control([kev], 0, 0)
+ try:
+ self._kqueue.control([kev], 0, 0)
+ except OSError:
+ # This can happen if the FD was closed since it
+ # was registered.
+ pass
if key.events & EVENT_WRITE:
kev = select.kevent(key.fd, select.KQ_FILTER_WRITE,
select.KQ_EV_DELETE)
- self._kqueue.control([kev], 0, 0)
+ try:
+ self._kqueue.control([kev], 0, 0)
+ except OSError:
+ # See comment above.
+ pass
return key
def select(self, timeout=None):
diff --git a/Lib/test/test_selectors.py b/Lib/test/test_selectors.py
--- a/Lib/test/test_selectors.py
+++ b/Lib/test/test_selectors.py
@@ -1,4 +1,5 @@
import errno
+import os
import random
import selectors
import signal
@@ -49,13 +50,17 @@
class BaseSelectorTestCase(unittest.TestCase):
+ def make_socketpair(self):
+ rd, wr = socketpair()
+ self.addCleanup(rd.close)
+ self.addCleanup(wr.close)
+ return rd, wr
+
def test_register(self):
s = self.SELECTOR()
self.addCleanup(s.close)
- rd, wr = socketpair()
- self.addCleanup(rd.close)
- self.addCleanup(wr.close)
+ rd, wr = self.make_socketpair()
key = s.register(rd, selectors.EVENT_READ, "data")
self.assertIsInstance(key, selectors.SelectorKey)
@@ -81,9 +86,7 @@
s = self.SELECTOR()
self.addCleanup(s.close)
- rd, wr = socketpair()
- self.addCleanup(rd.close)
- self.addCleanup(wr.close)
+ rd, wr = self.make_socketpair()
s.register(rd, selectors.EVENT_READ)
s.unregister(rd)
@@ -94,13 +97,51 @@
# unregister twice
self.assertRaises(KeyError, s.unregister, rd)
+ def test_unregister_after_fd_close(self):
+ s = self.SELECTOR()
+ self.addCleanup(s.close)
+ rd, wr = self.make_socketpair()
+ r, w = rd.fileno(), wr.fileno()
+ s.register(r, selectors.EVENT_READ)
+ s.register(w, selectors.EVENT_WRITE)
+ rd.close()
+ wr.close()
+ s.unregister(r)
+ s.unregister(w)
+
+ def test_unregister_after_fd_close_and_reuse(self):
+ s = self.SELECTOR()
+ self.addCleanup(s.close)
+ rd, wr = self.make_socketpair()
+ r, w = rd.fileno(), wr.fileno()
+ s.register(r, selectors.EVENT_READ)
+ s.register(w, selectors.EVENT_WRITE)
+ rd2, wr2 = self.make_socketpair()
+ rd.close()
+ wr.close()
+ os.dup2(rd2.fileno(), r)
+ os.dup2(wr2.fileno(), w)
+ self.addCleanup(os.close, r)
+ self.addCleanup(os.close, w)
+ s.unregister(r)
+ s.unregister(w)
+
+ def test_unregister_after_socket_close(self):
+ s = self.SELECTOR()
+ self.addCleanup(s.close)
+ rd, wr = self.make_socketpair()
+ s.register(rd, selectors.EVENT_READ)
+ s.register(wr, selectors.EVENT_WRITE)
+ rd.close()
+ wr.close()
+ s.unregister(rd)
+ s.unregister(wr)
+
def test_modify(self):
s = self.SELECTOR()
self.addCleanup(s.close)
- rd, wr = socketpair()
- self.addCleanup(rd.close)
- self.addCleanup(wr.close)
+ rd, wr = self.make_socketpair()
key = s.register(rd, selectors.EVENT_READ)
@@ -138,9 +179,7 @@
s = self.SELECTOR()
self.addCleanup(s.close)
- rd, wr = socketpair()
- self.addCleanup(rd.close)
- self.addCleanup(wr.close)
+ rd, wr = self.make_socketpair()
s.register(rd, selectors.EVENT_READ)
s.register(wr, selectors.EVENT_WRITE)
@@ -153,9 +192,7 @@
s = self.SELECTOR()
self.addCleanup(s.close)
- rd, wr = socketpair()
- self.addCleanup(rd.close)
- self.addCleanup(wr.close)
+ rd, wr = self.make_socketpair()
key = s.register(rd, selectors.EVENT_READ, "data")
self.assertEqual(key, s.get_key(rd))
@@ -167,9 +204,7 @@
s = self.SELECTOR()
self.addCleanup(s.close)
- rd, wr = socketpair()
- self.addCleanup(rd.close)
- self.addCleanup(wr.close)
+ rd, wr = self.make_socketpair()
keys = s.get_map()
self.assertFalse(keys)
@@ -194,9 +229,7 @@
s = self.SELECTOR()
self.addCleanup(s.close)
- rd, wr = socketpair()
- self.addCleanup(rd.close)
- self.addCleanup(wr.close)
+ rd, wr = self.make_socketpair()
s.register(rd, selectors.EVENT_READ)
wr_key = s.register(wr, selectors.EVENT_WRITE)
@@ -214,9 +247,7 @@
s = self.SELECTOR()
self.addCleanup(s.close)
- rd, wr = socketpair()
- self.addCleanup(rd.close)
- self.addCleanup(wr.close)
+ rd, wr = self.make_socketpair()
with s as sel:
sel.register(rd, selectors.EVENT_READ)
@@ -247,9 +278,7 @@
w2r = {}
for i in range(NUM_SOCKETS):
- rd, wr = socketpair()
- self.addCleanup(rd.close)
- self.addCleanup(wr.close)
+ rd, wr = self.make_socketpair()
s.register(rd, selectors.EVENT_READ)
s.register(wr, selectors.EVENT_WRITE)
readers.append(rd)
@@ -293,9 +322,7 @@
s = self.SELECTOR()
self.addCleanup(s.close)
- rd, wr = socketpair()
- self.addCleanup(rd.close)
- self.addCleanup(wr.close)
+ rd, wr = self.make_socketpair()
s.register(wr, selectors.EVENT_WRITE)
t = time()
@@ -322,9 +349,7 @@
s = self.SELECTOR()
self.addCleanup(s.close)
- rd, wr = socketpair()
- self.addCleanup(rd.close)
- self.addCleanup(wr.close)
+ rd, wr = self.make_socketpair()
orig_alrm_handler = signal.signal(signal.SIGALRM, lambda *args: None)
self.addCleanup(signal.signal, signal.SIGALRM, orig_alrm_handler)
@@ -364,16 +389,13 @@
for i in range(NUM_FDS // 2):
try:
- rd, wr = socketpair()
+ rd, wr = self.make_socketpair()
except OSError:
# too many FDs, skip - note that we should only catch EMFILE
# here, but apparently *BSD and Solaris can fail upon connect()
# or bind() with EADDRNOTAVAIL, so let's be safe
self.skipTest("FD limit reached")
- self.addCleanup(rd.close)
- self.addCleanup(wr.close)
-
try:
s.register(rd, selectors.EVENT_READ)
s.register(wr, selectors.EVENT_WRITE)
--
Repository URL: http://hg.python.org/cpython
More information about the Python-checkins
mailing list