On 17/01/2013 6:53 p.m., Alex Rousskov wrote:
On 01/16/2013 09:58 PM, Amos Jeffries wrote:
Whet I was trying to point out is that we have three race conditions
between timeout() and connect() Calls...
* connect() happens first and runs re-schedules while timeout() is queued.
- this is the case we are now ignoring. I'm fine with that.
Yes (a failed connect wins and retries).
* connect() happens first and runs doneConnection() while timeout() is
queued.
Yes (a failed connect wins and ends ConnOpener job).
- in this case doneConnecting() will cancel the timeout() queued Call.
There is no need to cancel any pending calls to ConnOpener. The async
call will simply not be delivered to the destroyed ConnOpener job. What
we do need is to cleanup anything we have allocated (that cleanup should
not be in the timeout callback or, at least, that cleanup should not be
exclusive to the timeout callback because the callback may never be called).
It is currently done by doneConnecting(). Which is the correct way to
exit ConnOpener with success/error status.
* timeout() happens first and runs doneConnecting() while connect() is
queued.
Yes (timeout wins and ends ConnOpener job).
- we are still connecting because the connect() Call has not run.
Yes, that is possible. We should tell Comm to stop connecting (if Comm
still does), to avoid wasting resources, but everything ought to work
correctly even if we do not tell Comm.
- we have no handle to the connect() AsyncCall stored so we can't
cancel it directly (bug?).
We do not need to cancel it (just like with the timeout call discussed
above). The async call will not be delivered to the destroyed ConnOpener
job).
Okay. If the Async code prevents the ConnOpener::connect() function
being run, then the original if() which stillConnecting() replaces there
is now obsolete.
Amos
