On 24/09/10 04:39, Alex Rousskov wrote:
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.
Ah thats what you meant. Connection is trying to stay as close to a dumb
storage type as possible.
The lookups fde state changes needed by connected() would make it
heavier. I'd avoid it's close() as well if I could.
NP: long-term the plan is to have TPROXY, TLS/SSL, EUI and IDENT to the
remote end of the new connection also started at the connected() point.
Based on Connection::flags.
With the equivalent client-facing ones in ConnAcceptor.
Amos
--
Please be using
Current Stable Squid 2.7.STABLE9 or 3.1.8
Beta testers wanted for 3.2.0.2
