fishy commented on a change in pull request #2200:
URL: https://github.com/apache/thrift/pull/2200#discussion_r464500154
##########
File path: lib/py/src/transport/TSocket.py
##########
@@ -74,7 +74,30 @@ def setHandle(self, h):
self.handle = h
def isOpen(self):
- return self.handle is not None
+ if self.handle is None:
+ return False
+
+ # this lets us cheaply see if the other end of the socket is still
+ # connected. if disconnected, we'll get EOF back (expressed as zero
+ # bytes of data) otherwise we'll get one byte or an error indicating
+ # we'd have to block for data.
+ #
+ # note that we're not doing this with socket.MSG_DONTWAIT because 1)
+ # it's linux-specific and 2) gevent-patched sockets hide EAGAIN from us
+ # when timeout is non-zero.
+ original_timeout = self.handle.gettimeout()
+ try:
+ self.handle.settimeout(0)
+ try:
+ peeked_bytes = self.handle.recv(1, socket.MSG_PEEK)
+ except (socket.error, OSError) as exc: # on modern python this is
just BlockingIOError
+ if exc.errno == errno.EWOULDBLOCK:
Review comment:
I think we actually need to check both EWOULDBLOCK and EAGAIN here? From
the man page of `recv`:
```
EAGAIN or EWOULDBLOCK
The socket is marked nonblocking and the receive operation
would block, or a receive timeout had been set and the timeout expired before
data was received. POSIX.1 allows either error to be returned for this case,
and does not require these constants to have the same value, so a portable
application should check for both possibilities.
```
Also should EOF be handled explicitly (return False) instead of just raise
again?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
