On May 8, 2009, at 2:48 PM, Rush Manbert wrote:
>
> On May 8, 2009, at 2:05 PM, Patrick Schlangen wrote:
>
>> Hi,
>>
>> the problem disappears if you define __FreeBSD__ in TSocket.cpp and
>> change
>>
>> virtual bool peek() {
>> /* shigin: see THRIFT-96 discussion */
>> if (rBase_ == rBound_) {
>> setReadBuffer(rBuf_.get(), transport_->read(rBuf_.get(),
>> rBufSize_));
>> }
>> return (rBound_ > rBase_);
>> }
>>
>> to
>>
>> virtual bool peek() {
>> return (rBound_ > rBase_ || transport_->peek());
>> }
>>
>> in TBufferTransports.h
>>
>> I don't know if this really fixes the issue or just hides it and I
>> don't know if I break anything by making this change as I don't
>> fully understand the code.
>>
>> I guess that MacOS's recv() implementation issues ECONNRESET the
>> second time it is called on a closed socket.
>> I'm going to test that.
>
> Hi Patrick,
>
> After your previous email I went and tested various flavors of
> TSocket close(). I tried all the permutations of SHUT_RD, SHUT_WR,
> SHUT_RDWR, and calling/not calling close on the client side. Nothing
> makes the problem disappear.
>
> But you seem to have hit on the real issue. First, the code that
> handles receiving ECONNRESET when the other side closes the socket
> is qualified by a #define that isn't generated by configure when run
> on the Mac. Second, the optimization in TBufferTransports.h looks
> like it shouldn't be there, or should conditionally call the
> underlying peek() on a Mac. I read through the THRIFT-96 discussion
> and it sounds like calling the underlying peek is not a problem. Too
> bad we hadn't come down this road before THRIFT-96 was accepted.
In fact, if you change the line in TSocket from this:
#if defined __FreeBSD__
to this:
#if defined __FreeBSD__ || defined __MACH__
you get the desired behavior at that level.
- Rush
|