This patch seeks to avoids race conditions between the AsyncCalls for
the pconn monitoring handlers and lookups pulling pconn up for use.
It adds checks for the read handler, re-implementing the safety feature
which appears to have been attempted and failed at by the earlier use of
read_pending flag. Which _actually_ means SSL code is writing something
that will cause future reads.
It adds checks for the timeout handler, as discussed in bug 3281,
preventing the related type of race from the timeout handler.
Amos
--
Please be using
Current Stable Squid 2.7.STABLE9 or 3.1.15
Beta testers wanted for 3.2.0.10
=== modified file 'src/pconn.cc'
--- src/pconn.cc 2011-07-08 00:12:40 +0000
+++ src/pconn.cc 2011-09-01 10:56:40 +0000
@@ -194,22 +194,34 @@
commSetConnTimeout(conn, Config.Timeout.pconn, timeoutCall);
}
+/// Determine whether an entry in the idle list is available for use.
+/// Returns false if entry is unset, closed, or appears timed out.
+bool
+IdleConnList::isAvailable(int i) const
+{
+ const Comm::ConnectionPointer &conn = theList_[i];
+
+ // connection already closed. useless.
+ if (!Comm::IsConnOpen(conn))
+ return false;
+
+ // our connection timeout handler is scheduled to run already. unsafe
+ if (fd_table[conn->fd].timeoutHandler == NULL)
+ return false;
+
+ // our connection early-read/close handler is scheduled to run already. unsafe
+ if (!COMMIO_FD_READCB(conn->fd)->active())
+ return false;
+
+ return true;
+}
+
Comm::ConnectionPointer
IdleConnList::pop()
{
for (int i=size_-1; i>=0; i--) {
- // Is the FD pending completion of the closure callback?
- // this flag is set while our early-read/close handler is
- // waiting for a remote response. It gets unset when the
- // handler is scheduled.
- //The following check is disabled for now until we have a
- // correct implementation of the read_pending flag
- //if (!fd_table[theList_[i]->fd].flags.read_pending)
- // continue;
-
- // connection already closed. useless.
- if (!Comm::IsConnOpen(theList_[i]))
+ if (!isAvailable(i))
continue;
// finally, a match. pop and return it.
@@ -242,17 +254,7 @@
for (int i=size_-1; i>=0; i--) {
- // Is the FD pending completion of the closure callback?
- // this flag is set while our early-read/close handler is
- // waiting for a remote response. It gets unset when the
- // handler is scheduled.
- //The following check is disabled for now until we have a
- // correct implementation of the read_pending flag
- //if (!fd_table[theList_[i]->fd].flags.read_pending)
- // continue;
-
- // connection already closed. useless.
- if (!Comm::IsConnOpen(theList_[i]))
+ if (!isAvailable(i))
continue;
// local end port is required, but dont match.
=== modified file 'src/pconn.h'
--- src/pconn.h 2011-06-17 06:04:05 +0000
+++ src/pconn.h 2011-09-01 10:37:31 +0000
@@ -55,6 +55,7 @@
void closeN(size_t count);
private:
+ bool isAvailable(int i) const;
bool removeAt(int index);
int findIndexOf(const Comm::ConnectionPointer &conn) const;
static IOCB Read;
