Yep, that sounds like the right approach to me. Now that I think about it, all methods of TFramed/Buffered transports that call the underlying transport should have try/catch blocks for TTransportException, and either handle or cascade the Exception appropriately.
-----Original Message----- From: Erik Frey [mailto:[EMAIL PROTECTED] Sent: Tuesday, October 14, 2008 10:52 AM To: [email protected] Subject: Re: invalid state in TFramed/BufferedTransport Very well, then its safe to argue that a failed flush means game over: I'll submit a patch that forces buffered transports to clear their write buffers regardless of the flush()'s success. Erik On Tuesday 14 October 2008 18:42:26 Mark Slee wrote: > I think any retry behaviors should be encoded into the semantics of > the transport. If you want retries, then you should configure the > transport as such, and a single call to flush() should retry on > failure if desired. This keeps the error-handling state-management > confined to the transport internals, and doesn't leak the retry > abstraction out into application code. > > Making repeated calls seems like a slipperly slope. My 2c. > > Cheers, > mcslee > > -----Original Message----- > From: Erik Frey [mailto:[EMAIL PROTECTED] > Sent: Tuesday, October 14, 2008 9:07 AM > To: [email protected] > Cc: Bryan Duxbury > Subject: Re: invalid state in TFramed/BufferedTransport > > Okay -- my only concern is whether the ability to retry is inferred in > a method like flush(), if a flush() fails, should the transport > preserve its state so that I can try flushing again? > > If so, this fix would break that property. > > Erik > > On Tuesday 14 October 2008 15:56:20 Bryan Duxbury wrote: > > I think it makes the most sense for a failed flush() to clear the > > write buffer. That way, you get the most immediate adjustment > > towards consistency, and since you're closing, the data is clearly > > unrecoverable anyway. > > > > Another approach would be to initialize the buffer in open(). > > > > -Bryan > > > > On Oct 14, 2008, at 5:56 AM, Erik Frey wrote: > > > Before I submit a patch to TBuffered/FramedTransport I'd like some > > > input as to which is the better fix: > > > > > > Right now, when TFramedTransport's underlying transport fails > > > during > > > > > > a flush(), TFramedTransport enters an irrecoverable state with > > > unflushed data in its write buffer. > > > > > > This can cause disastrous results in a naive client implementation: > > > > > > try > > > { > > > results = client.doSomething(first_param); } catch > > > (TTransportException &) { socket.close(); socket.open(host, port); > > > } > > > > > > results = client.doSomething(second_param); > > > > > > The second time the client is used, it will send the old buffered > > > request and interpret the wrong response! We've seen this problem > > > in both the c > > > ++ and > > > php libs. > > > > > > There are two ways to fix this: > > > > > > The first is for TFramedTransport to clear its write buffer for a > > > flush regardless of whether the underlying transport succeeds in > > > its > > > > > > write/flush. > > > > > > The second is for TFramedTransport's close() method to clear its > > > buffer regardless of whether the underlying flush() succeeds. In > > > this case, the implementer will have to close/open the framed > > > transport as well as the socket, but at least this is clear and > > > documentable behaviour. > > > > > > Thoughts? > > > > > > Erik
