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
