THRIFT-5813: Close the socket in TSocket.isOpen() when peek() fails
Client: py
diff --git a/lib/py/src/transport/TSocket.py b/lib/py/src/transport/TSocket.py
index 459f043..51ea7e3 100644
--- a/lib/py/src/transport/TSocket.py
+++ b/lib/py/src/transport/TSocket.py
@@ -90,18 +90,22 @@
self.handle.settimeout(0)
try:
peeked_bytes = self.handle.recv(1, socket.MSG_PEEK)
+ # the length will be zero if we got EOF (indicating connection closed)
+ if len(peeked_bytes) == 1:
+ return True
except (socket.error, OSError) as exc: # on modern python this is just BlockingIOError
if exc.errno in (errno.EWOULDBLOCK, errno.EAGAIN):
return True
- return False
except ValueError:
# SSLSocket fails on recv with non-zero flags; fallback to the old behavior
return True
finally:
self.handle.settimeout(original_timeout)
- # the length will be zero if we got EOF (indicating connection closed)
- return len(peeked_bytes) == 1
+ # The caller may assume that after isOpen() returns False, calling close()
+ # is not needed, so it is safer to close the socket here.
+ self.close()
+ return False
def setTimeout(self, ms):
if ms is None:
diff --git a/lib/py/test/test_socket.py b/lib/py/test/test_socket.py
index ab56a6e..13c1d51 100644
--- a/lib/py/test/test_socket.py
+++ b/lib/py/test/test_socket.py
@@ -67,9 +67,11 @@
# once the server side closes, it no longer shows open
acc.client.close() # this also blocks until the other thread is done
acc.close()
- self.assertFalse(sock.isOpen())
- sock.close()
+ self.assertIsNotNone(sock.handle)
+ self.assertFalse(sock.isOpen())
+ # after isOpen() returned False the socket should be closed (THRIFT-5813)
+ self.assertIsNone(sock.handle)
if __name__ == "__main__":