On 09/23/2010 09:46 AM, Amos Jeffries wrote:
On 23/09/10 04:49, Alex Rousskov wrote:
On 09/22/2010 08:28 AM, Amos Jeffries wrote:
On 21/09/10 10:00, Alex Rousskov wrote:
On 09/19/2010 05:35 AM, Amos Jeffries wrote:
Patch including the changes requested to date attached.
AFAIK, comm_close() will remove those handlers automagically because it
will call them with COMM_ERR_CLOSING. However, our code needs to make
sure that it can handle such callbacks until swanSong() is called.
K. Thats what I'm slightly worried about. This is inside swanSong. So by
the time this set of comm_close scheduled ones occur the swanSong is
over and done with. Is the cancel() enough to make dialing drop and skip?
connect() which they all lead back to starts with a Must() which will
trigger after swanSong() even if the job object itself is still alive.
It won't loop more than once, but scheduling is very a sub-optimal way
to kill them.
Nothing can happen to the job after swanSong() because the job is
deleted after it signs its last song. A dead job cannot receive any
callbacks. You may but do not have to cancel Comm callbacks because they
will not reach the job anyway. Calling comm_close should be enough to
keep Comm state consistent.
Specifically:
+ // rollback what we can from the job state
+ if (conn_ != NULL && conn_->isOpen()) {
.
+ // drop any handlers now to save a lot of cycles later
+ commSetSelect(conn_->fd, COMM_SELECT_WRITE, NULL, NULL, 0);
_comm_close does that if there is a write callback.
+ commSetTimeout(conn_->fd, -1, NULL, NULL);
_comm_close does that.
+ // it never reached fully open, so abort the FD
+ conn_->close();
That must be enough.
The only remaining thing is moving Comm::ConnOpener::connected() to
Connection::connected() but that is minor. Could be useful for those
cases were we still connect without using your new code though.
Thank you,
Alex.
