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
