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

Rainer Weikusat <rweikusat_at_mobileactivedefense.com> writes:

[...]

>> In your latest patch, please move fd cleanup from timeout() and from
>> doneConnecting() into a dedicated method and call that method from
>> swanSong() if temporaryFd_ is still set there. Do not call it from
>> doneConnecting() or anywhere else. Remember that a job may end for
>> reasons other than some timeout or I/O. swanSong() is the cleanup method.
>>
>> Please do not forget to set calls.earlyAbort to NULL after canceling it
>> in the fd cleanup method.
>
> What precisely constitutes 'fd_cleanup'?

[...]

>>> + ptr = static_cast<Pointer *>(F->write_data);
>>> + delete ptr;
>>
>> This is not going to work well. F->write_data now points to deleted
>> memory. If you absolutely have to do this, please set it to NULL after
>> delete.
>
> Insofar my understanding goes, the

Addition: Should swanSong be invoked (which invokes doneConnecting for
'low-level cleanup', possibly twice in a row) while the write handler
is still pending, the ConnOpener object will very probably be leaked,
too, and I remember that I got a valgrind output which looked as if it
confirmed this. Consequently, cleaning up/ fixing the 'cleanup code'
seems necessary as well and I'll happily to that as well since it is
another bug affecting me. But I would then suggest to do this in a
separate patch (details to be determined) which should either go
before the timeout changes or come after them.
Received on Thu Jan 17 2013 - 22:17:56 MST