[THRIFT-869] TSocket.py on Mac (and FreeBSD) doesn't handle ECONNRESET from recv()

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 0.4
• Fix Version/s: 0.5
• Component/s: Python - Library

Environment

Mac OS X (Darwin).
Also presumably on FreeBSD (although I don't have a FreeBSD system available for testing).

• Patch Info: Patch Available
• Assignee: Steven Knight
• Reporter: Steven Knight
• Created: 25/Aug/10 22:11
• Updated: 01/Nov/11 02:51
• Resolved: 30/Aug/10 21:58

Description

Use of Python's TSocket.read() will produce a lot of stack traces like the following:

Traceback (most recent call last):
File "net/mtvhome46.nfs/vol/mtvhome46/sgk/src/presto1/presto/third_party/thrift/Darwin/lib/python2.5/site-packages/thrift/server/TServer.py", line 83, in serve
File "/net/mtvhome46.nfs/vol/mtvhome46/sgk/src/presto1/presto/build/Darwin/lib/presto/pylib/presto/PrestoService.py", line 94, in process
(name, type, seqid) = iprot.readMessageBegin()
File "net/mtvhome46.nfs/vol/mtvhome46/sgk/src/presto1/presto/third_party/thrift/Darwin/lib/python2.5/site-packages/thrift/protocol/TBinaryProtocol.py", line 126, in readMessageBegin
File "net/mtvhome46.nfs/vol/mtvhome46/sgk/src/presto1/presto/third_party/thrift/Darwin/lib/python2.5/site-packages/thrift/protocol/TBinaryProtocol.py", line 203, in readI32
File "net/mtvhome46.nfs/vol/mtvhome46/sgk/src/presto1/presto/third_party/thrift/Darwin/lib/python2.5/site-packages/thrift/transport/TTransport.py", line 58, in readAll
File "net/mtvhome46.nfs/vol/mtvhome46/sgk/src/presto1/presto/third_party/thrift/Darwin/lib/python2.5/site-packages/thrift/transport/TTransport.py", line 155, in read
File "net/mtvhome46.nfs/vol/mtvhome46/sgk/src/presto1/presto/third_party/thrift/Darwin/lib/python2.5/site-packages/thrift/transport/TSocket.py", line 92, in read
error: (54, 'Connection reset by peer')

The underlying issue is explained by the following #ifdef'ed code in the C++ implementation, line 305 in the 0.4.0 version of lib/cpp/src/transport/TSocket.cpp:

#if defined _FreeBSD_ || defined _MACH_
if (errno_copy == ECONNRESET)

{ /* shigin: freebsd doesn't follow POSIX semantic of recv and fails with * ECONNRESET if peer performed shutdown */ close(); return 0; }

#endif

TSocket.py doesn't have any corresponding logic to handle this deviation from POSIX semantics. I'll attach a patch that implements logic similar to the C++ code, and which fixes the problem for me.

The patch might want some clean up for efficiency. I was cautious and had it check (every time an exception is handled) for whether there is, in fact, an errno.ECONNRESET attribute, on the off-chance that some other Python platform out there might not provide it. I also have it check (only if ECONNRESET is received) for whether we're actually on a Darwin or FreeBSD system, to avoid swallowing the exception on other, well-behaved systems.