Silently ignore unregistering closed files. Fixes issue 19876. With docs and slight test refactor.

This commit is contained in:
Guido van Rossum 2013-12-07 15:57:01 -08:00
parent 647cd87169
commit 9710ff04ac
3 changed files with 129 additions and 48 deletions

View file

@ -102,7 +102,8 @@ below:
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 @@ below:
*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)

View file

@ -25,6 +25,9 @@ def _fileobj_to_fd(fileobj):
Returns:
corresponding file descriptor
Raises:
ValueError if the object is invalid
"""
if isinstance(fileobj, int):
fd = fileobj
@ -55,7 +58,8 @@ def __len__(self):
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 @@ def register(self, fileobj, events, data=None):
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 @@ def unregister(self, fileobj):
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 @@ def modify(self, fileobj, events, data=None):
Returns:
SelectorKey instance
Raises:
Anything that unregister() or register() raises
"""
self.unregister(fileobj)
return self.register(fileobj, events, data)
@ -177,22 +200,41 @@ def __init__(self):
# 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 unregister(self, fileobj):
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 register(self, fileobj, events, data=None):
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 @@ def unregister(self, fileobj):
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):

View file

@ -1,4 +1,5 @@
import errno
import os
import random
import selectors
import signal
@ -49,13 +50,17 @@ def find_ready_matching(ready, flag):
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 @@ def test_unregister(self):
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 @@ def test_unregister(self):
# 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 @@ def test_close(self):
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 @@ def test_get_key(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.assertEqual(key, s.get_key(rd))
@ -167,9 +204,7 @@ def test_get_map(self):
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 @@ def test_select(self):
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 @@ def test_context_manager(self):
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 @@ def test_selector(self):
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 @@ def test_timeout(self):
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 @@ def test_select_interrupt(self):
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 @@ def test_above_fd_setsize(self):
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)