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
