Skip to content

Commit

Permalink
THRIFT-5813: (Python) close the socket in isOpen() when peek() fails
Browse files Browse the repository at this point in the history
  • Loading branch information
csringhofer committed Aug 23, 2024
1 parent cb9cead commit a42f906
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 5 deletions.
10 changes: 7 additions & 3 deletions lib/py/src/transport/TSocket.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,18 +90,22 @@ def isOpen(self):
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:
Expand Down
6 changes: 4 additions & 2 deletions lib/py/test/test_socket.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,11 @@ def test_isOpen_checks_for_readability(self):
# 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__":
Expand Down

0 comments on commit a42f906

Please sign in to comment.