From 5aeb0098d2347984f3a89cf036c305edd2b8382b Mon Sep 17 00:00:00 2001 From: bjmb Date: Sat, 15 Jul 2017 18:49:17 -0400 Subject: [PATCH 1/6] Added failing tests for asyncore --- Lib/test/test_asyncore.py | 52 ++++++++++++++++++++++++++++++++++----- 1 file changed, 46 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_asyncore.py b/Lib/test/test_asyncore.py index dc2f716e0bb828..dc67018809dc8f 100644 --- a/Lib/test/test_asyncore.py +++ b/Lib/test/test_asyncore.py @@ -499,6 +499,12 @@ def handle_connect(self): pass +class SendHandler(BaseTestHandler): + def __init__(self, conn): + BaseTestHandler.__init__(self, conn) + self.send(b'x' * 1024) + + class BaseTestAPI: def tearDown(self): @@ -576,12 +582,7 @@ class TestClient(BaseClient): def handle_read(self): self.flag = True - class TestHandler(BaseTestHandler): - def __init__(self, conn): - BaseTestHandler.__init__(self, conn) - self.send(b'x' * 1024) - - server = BaseServer(self.family, self.addr, TestHandler) + server = BaseServer(self.family, self.addr, SendHandler) client = TestClient(self.family, server.address) self.loop_waiting_for_flag(client) @@ -800,6 +801,45 @@ def test_quick_connect(self): if t.is_alive(): self.fail("join() timed out") + @unittest.expectedFailure + def test_map_altered_in_loop(self): + family = self.family + fail = self.fail + + class PoisonedClient(BaseClient): + """ + This dispatcher is created after closing a writable one + """ + def handle_write(self): + fail("Attempt to call handle_write on the wrong client") + + # The dispatcher is not writable and therefore handle_write shouldn't be called + def writable(self): + return False + + class ManagerClient(BaseClient): + + def __init__(self, family, address): + BaseClient.__init__(self, family, address) + self.old_client = BaseClient(family, address) + + def handle_write(self): + old_fd = self.old_client._fileno + self.old_client.close() + # This trusts that the fd of this client new + # will be the same to the one just closed + new_client = PoisonedClient(family, server.address) + if new_client._fileno != old_fd: + raise unittest.SkipTest("The test is meaningful only if the fd for the old and " + "the new dispatcher are the same") + + self.flag = True + + server = BaseServer(self.family, self.addr, SendHandler) + manager = ManagerClient(self.family, server.address) + self.loop_waiting_for_flag(manager) + + class TestAPI_UseIPv4Sockets(BaseTestAPI): family = socket.AF_INET addr = (support.HOST, 0) From ab29440688b7d87d58b64963f90da14329fe169a Mon Sep 17 00:00:00 2001 From: bjmb Date: Tue, 18 Jul 2017 20:41:07 -0400 Subject: [PATCH 2/6] Fix asyncore --- Lib/asyncore.py | 35 ++++++++++++++++++++++------------- Lib/test/test_asyncore.py | 1 - 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/Lib/asyncore.py b/Lib/asyncore.py index 705e4068130325..639cc4ca2edbdf 100644 --- a/Lib/asyncore.py +++ b/Lib/asyncore.py @@ -131,33 +131,30 @@ def poll(timeout=0.0, map=None): is_r = obj.readable() is_w = obj.writable() if is_r: - r.append(fd) + r.append(obj) # accepting sockets should not be writable if is_w and not obj.accepting: - w.append(fd) + w.append(obj) if is_r or is_w: - e.append(fd) + e.append(obj) if [] == r == w == e: time.sleep(timeout) return r, w, e = select.select(r, w, e, timeout) - for fd in r: - obj = map.get(fd) - if obj is None: + for obj in r: + if obj.fileno() is None: continue read(obj) - for fd in w: - obj = map.get(fd) - if obj is None: + for obj in w: + if obj.fileno() is None: continue write(obj) - for fd in e: - obj = map.get(fd) - if obj is None: + for obj in e: + if obj.fileno() is None: continue _exception(obj) @@ -180,12 +177,18 @@ def poll2(timeout=0.0, map=None): if flags: pollster.register(fd, flags) + ready = [] r = pollster.poll(timeout) for fd, flags in r: obj = map.get(fd) if obj is None: continue - readwrite(obj, flags) + ready.append((obj, flags)) + + for obj, flags in ready: + if obj.fileno() is not None: + readwrite(obj, flags) + poll3 = poll2 # Alias for backward compatibility @@ -399,6 +402,9 @@ def close(self): if why.args[0] not in (ENOTCONN, EBADF): raise + def fileno(self): + return self.socket.fileno() + # log and log_info may be overridden to provide more sophisticated # logging and warning methods. In general, log is for 'hit' logging # and 'log_info' is for informational, warning and error logging. @@ -642,3 +648,6 @@ def set_file(self, fd): self.socket = file_wrapper(fd) self._fileno = self.socket.fileno() self.add_channel() + + def fileno(self): + return self.socket.fileno() diff --git a/Lib/test/test_asyncore.py b/Lib/test/test_asyncore.py index dc67018809dc8f..416e185384a2f7 100644 --- a/Lib/test/test_asyncore.py +++ b/Lib/test/test_asyncore.py @@ -801,7 +801,6 @@ def test_quick_connect(self): if t.is_alive(): self.fail("join() timed out") - @unittest.expectedFailure def test_map_altered_in_loop(self): family = self.family fail = self.fail From 33f1819398a4e97bd7a34a2177a1c9bcab5d8651 Mon Sep 17 00:00:00 2001 From: bjmb Date: Wed, 19 Jul 2017 09:25:09 -0400 Subject: [PATCH 3/6] Fixed review comments --- Lib/asyncore.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/Lib/asyncore.py b/Lib/asyncore.py index 639cc4ca2edbdf..b7e5bb1d11bd66 100644 --- a/Lib/asyncore.py +++ b/Lib/asyncore.py @@ -144,17 +144,17 @@ def poll(timeout=0.0, map=None): r, w, e = select.select(r, w, e, timeout) for obj in r: - if obj.fileno() is None: + if obj.fileno() is -1: continue read(obj) for obj in w: - if obj.fileno() is None: + if obj.fileno() is -1: continue write(obj) for obj in e: - if obj.fileno() is None: + if obj.fileno() is -1: continue _exception(obj) @@ -180,13 +180,10 @@ def poll2(timeout=0.0, map=None): ready = [] r = pollster.poll(timeout) for fd, flags in r: - obj = map.get(fd) - if obj is None: - continue - ready.append((obj, flags)) + ready.append((map[fd], flags)) for obj, flags in ready: - if obj.fileno() is not None: + if obj.fileno() is not -1: readwrite(obj, flags) @@ -397,13 +394,14 @@ def close(self): self.del_channel() if self.socket is not None: try: + self._fileno = -1 self.socket.close() except OSError as why: if why.args[0] not in (ENOTCONN, EBADF): raise def fileno(self): - return self.socket.fileno() + return self._fileno # log and log_info may be overridden to provide more sophisticated # logging and warning methods. In general, log is for 'hit' logging @@ -650,4 +648,4 @@ def set_file(self, fd): self.add_channel() def fileno(self): - return self.socket.fileno() + return self._fileno From 8d9aec1e57f3ab6bee7b2b6fcf2a9164e919e20e Mon Sep 17 00:00:00 2001 From: bjmb Date: Wed, 19 Jul 2017 12:14:21 -0400 Subject: [PATCH 4/6] Use closing variable --- Lib/asyncore.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/Lib/asyncore.py b/Lib/asyncore.py index b7e5bb1d11bd66..cfcdb06f4208ba 100644 --- a/Lib/asyncore.py +++ b/Lib/asyncore.py @@ -144,17 +144,17 @@ def poll(timeout=0.0, map=None): r, w, e = select.select(r, w, e, timeout) for obj in r: - if obj.fileno() is -1: + if obj.closing: continue read(obj) for obj in w: - if obj.fileno() is -1: + if obj.closing: continue write(obj) for obj in e: - if obj.fileno() is -1: + if obj.closing: continue _exception(obj) @@ -177,13 +177,13 @@ def poll2(timeout=0.0, map=None): if flags: pollster.register(fd, flags) - ready = [] r = pollster.poll(timeout) + ready = [] for fd, flags in r: ready.append((map[fd], flags)) for obj, flags in ready: - if obj.fileno() is not -1: + if not obj.closing: readwrite(obj, flags) @@ -388,20 +388,23 @@ def recv(self, buffer_size): raise def close(self): + if self.closing: + return + self.closing = True + self.connected = False self.accepting = False self.connecting = False self.del_channel() if self.socket is not None: try: - self._fileno = -1 self.socket.close() except OSError as why: if why.args[0] not in (ENOTCONN, EBADF): raise def fileno(self): - return self._fileno + return self.socket.fileno() # log and log_info may be overridden to provide more sophisticated # logging and warning methods. In general, log is for 'hit' logging @@ -648,4 +651,4 @@ def set_file(self, fd): self.add_channel() def fileno(self): - return self._fileno + return self.socket.fileno() From d9dfa940d71cf25b26697767a0332a4010398d1d Mon Sep 17 00:00:00 2001 From: bjmb Date: Wed, 19 Jul 2017 16:00:35 -0400 Subject: [PATCH 5/6] Review comments --- Lib/asyncore.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Lib/asyncore.py b/Lib/asyncore.py index cfcdb06f4208ba..41783ab05bb466 100644 --- a/Lib/asyncore.py +++ b/Lib/asyncore.py @@ -404,6 +404,8 @@ def close(self): raise def fileno(self): + if self.socket is None: + raise socket.error(EBADF, 'Bad file descriptor') return self.socket.fileno() # log and log_info may be overridden to provide more sophisticated @@ -649,6 +651,3 @@ def set_file(self, fd): self.socket = file_wrapper(fd) self._fileno = self.socket.fileno() self.add_channel() - - def fileno(self): - return self.socket.fileno() From 1b692ed5c33dc74d734c9fd16819d976fb7fec08 Mon Sep 17 00:00:00 2001 From: bjmb Date: Thu, 20 Jul 2017 11:14:57 -0400 Subject: [PATCH 6/6] Simplified loop --- Lib/asyncore.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Lib/asyncore.py b/Lib/asyncore.py index 41783ab05bb466..96ab9d8ecd9bf0 100644 --- a/Lib/asyncore.py +++ b/Lib/asyncore.py @@ -178,9 +178,7 @@ def poll2(timeout=0.0, map=None): pollster.register(fd, flags) r = pollster.poll(timeout) - ready = [] - for fd, flags in r: - ready.append((map[fd], flags)) + ready = [(map[fd], flags) for fd, flags in r] for obj, flags in ready: if not obj.closing: