Re: [PATCH] Move timeout handling from ConnOpener::connect to ConnOpener::timeout

On 18/01/2013 2:51 p.m., Alex Rousskov wrote:
> On 01/17/2013 02:53 PM, Rainer Weikusat wrote:
>> Alex Rousskov <rousskov_at_measurement-factory.com> writes:
>>> On 01/17/2013 10:56 AM, Rainer Weikusat wrote:
>> Then, the ::connect
>> method (or some code running on behalf of that) would close
>> temporaryFd_ before scheduling a 'delayed connect retry' which would
>> open a new socket. A ::timeout running then won't need to free
>> write_data and won't have to cancel the early_abort call.
> Agreed.
>
>> If the write handler was scheduled
>> by the time timout runs, this happened because 'something definite' is
>> known about the state of the connection attempt: Either, it suceeded
>> or the OS returned an error. Throwing this 'real information' away in
>> order to save a line of code seems not right to me even if it is a
>> fringe case of a fringe case.
> As I said, one would not be just saving lines of code. One would be most
> likely introducing bugs (that is exactly what happened in the past with
> this code). We have at most two "real information" pieces: a timeout and
> I/O status. The first to get to ConnOpener wins.

We have the problem that we do not know whether the future scheduled
::connect was scheduled with a success (usually is) or a IN_PROGRESS
which would re-start another in-progress event cycle. The Job won't know
that datum until that scheduled Call is run.
This is the exact edge case which the existing trunk code was written to
handle and did not work because the OS connect() handling has proven to
be unreliable. Where timeout needs to abort, even if ::connect() has
succeeded but taken 1ms too long (with a scheduled Call). Still a Job
exit on timeout error in the new design you are writing.

Amos
Received on Fri Jan 18 2013 - 05:13:33 MST