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

["David Reiss (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/THRIFT-869?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12903121#action_12903121 ] 

David Reiss commented on THRIFT-869:
------------------------------------

I don't think you need to check whether ECONNRESET is defined.  Can you drop that test and also do the imports at global scope?  Also, can you attach a unified diff?

> TSocket.py on Mac (and FreeBSD) doesn't handle ECONNRESET from recv()
> ---------------------------------------------------------------------
>
>                 Key: THRIFT-869
>                 URL: https://issues.apache.org/jira/browse/THRIFT-869
>             Project: Thrift
>          Issue Type: Bug
>          Components: Python - Library
>    Affects Versions: 0.4
>         Environment: Mac OS X (Darwin).
> Also presumably on FreeBSD (although I don't have a FreeBSD system available for testing).
>            Reporter: Steven Knight
>         Attachments: TSocket.ECONNRESET.patch
>
>
> 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.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.