Since 1xx handing went in HttpRequest has had two links to the one
ConnStateData managing its client connection.
* Rename the 1xx link to clientConnectionManager (since it is not
actually the connection, but the manager object controlling the FD usage
and stats.
* Convert the pinning code to using the permanent
clientConnectionManager link instead of a temporary pinned_connection link.
This moves all connection pinning state fully into the ConnStateData
manager objects scope.
Side changes that appear to be buggy code previously:
* do not alter pinning state at the point where the pinned connection
is about to start being used. Changes are only relevant at the point of
pinning or unpinning.
* unpin operation now closes the Server FD if still open. Previously
there was the possibility that some code paths would leave server FD
open and pconn it. (especially since the above mentioned state
alteration cleared the "pinned" flag).
Is there anyone using pinning able to assist test this please?
Amos
--
Please be using
Current Stable Squid 2.7.STABLE9 or 3.1.11
Beta testers wanted for 3.2.0.4
=== modified file 'src/HttpRequest.cc'
--- src/HttpRequest.cc 2011-02-07 10:27:53 +0000
+++ src/HttpRequest.cc 2011-02-10 12:42:29 +0000
@@ -87,7 +87,6 @@
#if USE_AUTH
auth_user_request = NULL;
#endif
- pinned_connection = NULL;
port = 0;
canonical = NULL;
memset(&flags, '\0', sizeof(flags));
@@ -157,9 +156,6 @@
range = NULL;
}
- if (pinned_connection)
- cbdataReferenceDone(pinned_connection);
-
myportname.clean();
tag.clean();
@@ -631,9 +627,10 @@
#if USE_AUTH
auth_user_request = aReq->auth_user_request;
#endif
- if (aReq->pinned_connection) {
- pinned_connection = cbdataReference(aReq->pinned_connection);
- }
+// if (aReq->pinnedConnection()) {
+// pinned_connection = cbdataReference(aReq->pinned_connection);
+// }
+ clientConnectionManager = aReq->clientConnectionManager;
return true;
}
=== modified file 'src/HttpRequest.h'
--- src/HttpRequest.h 2011-02-07 10:27:53 +0000
+++ src/HttpRequest.h 2011-02-10 11:01:09 +0000
@@ -143,12 +143,6 @@
char host[SQUIDHOSTNAMELEN];
int host_is_numeric;
- /***
- * The client side connection data of pinned connections for the client side
- * request related objects
- */
- ConnStateData *pinned_connection;
-
#if USE_ADAPTATION
mutable Adaptation::History::Pointer adaptHistory_; ///< per-HTTP transaction info
#endif
@@ -246,20 +240,18 @@
static HttpRequest * CreateFromUrl(char * url);
- void setPinnedConnection(ConnStateData *conn) {
- pinned_connection = cbdataReference(conn);
- }
-
ConnStateData *pinnedConnection() {
- return pinned_connection;
- }
-
- void releasePinnedConnection() {
- cbdataReferenceDone(pinned_connection);
- }
-
- /// client-side conn manager, if known; used for 1xx response forwarding
- CbcPointer<ConnStateData> clientConnection;
+ if (clientConnectionManager.valid() && clientConnectionManager->pinning.pinned)
+ return clientConnectionManager.get();
+ return NULL;
+ }
+
+ /**
+ * The client connection manager, if known;
+ * Used for any response actions needed directly to the client.
+ * ie 1xx forwarding or connection pinning state changes
+ */
+ CbcPointer<ConnStateData> clientConnectionManager;
int64_t getRangeOffsetLimit(); /* the result of this function gets cached in rangeOffsetLimit */
=== modified file 'src/client_side.cc'
--- src/client_side.cc 2011-02-07 10:27:53 +0000
+++ src/client_side.cc 2011-02-10 12:46:33 +0000
@@ -4005,7 +4005,8 @@
* connection has gone away */
}
-void ConnStateData::pinConnection(int pinning_fd, HttpRequest *request, struct peer *aPeer, bool auth)
+void
+ConnStateData::pinConnection(int pinning_fd, HttpRequest *request, struct peer *aPeer, bool auth)
{
fde *f;
char desc[FD_DESC_SZ];
@@ -4039,7 +4040,8 @@
}
-int ConnStateData::validatePinnedConnection(HttpRequest *request, const struct peer *aPeer)
+int
+ConnStateData::validatePinnedConnection(HttpRequest *request, const struct peer *aPeer)
{
bool valid = true;
if (pinning.fd < 0)
@@ -4059,21 +4061,15 @@
}
if (!valid) {
- int pinning_fd=pinning.fd;
- /* The pinning info is not safe, remove any pinning info*/
+ /* The pinning info is not safe, remove any pinning info */
unpinConnection();
-
- /* also close the server side socket, we should not use it for invalid/unauthenticated
- requests...
- */
- comm_close(pinning_fd);
- return -1;
}
return pinning.fd;
}
-void ConnStateData::unpinConnection()
+void
+ConnStateData::unpinConnection()
{
if (pinning.peer)
cbdataReferenceDone(pinning.peer);
@@ -4082,6 +4078,8 @@
comm_remove_close_handler(pinning.fd, pinning.closeHandler);
pinning.closeHandler = NULL;
}
+ /// also close the server side socket, we should not use it for any future requests...
+ comm_close(pinning.fd);
pinning.fd = -1;
safe_free(pinning.host);
}
=== modified file 'src/client_side_reply.cc'
--- src/client_side_reply.cc 2011-02-08 01:12:42 +0000
+++ src/client_side_reply.cc 2011-02-10 11:49:00 +0000
@@ -269,7 +269,7 @@
http->storeEntry(entry);
assert(http->out.offset == 0);
- http->request->clientConnection = http->getConn();
+ http->request->clientConnectionManager = http->getConn();
/*
* A refcounted pointer so that FwdState stays around as long as
@@ -660,7 +660,7 @@
if (http->flags.internal)
r->protocol = PROTO_INTERNAL;
- r->clientConnection = http->getConn();
+ r->clientConnectionManager = http->getConn();
/** Start forwarding to get the new object from network */
FwdState::fwdStart(http->getConn() != NULL ? http->getConn()->fd : -1,
=== modified file 'src/client_side_request.cc'
--- src/client_side_request.cc 2011-02-07 10:27:53 +0000
+++ src/client_side_request.cc 2011-02-10 12:04:36 +0000
@@ -798,7 +798,8 @@
} else {
request->flags.connection_proxy_auth = 1;
}
- request->setPinnedConnection(http_conn);
+ // These should already be linked correctly.
+ assert(request->clientConnectionManager == http_conn);
}
}
@@ -831,7 +832,8 @@
}
}
if (may_pin && !request->pinnedConnection()) {
- request->setPinnedConnection(http->getConn());
+ // These should already be linked correctly. Just need the ServerConnection to pinn.
+ assert(request->clientConnectionManager == http_conn);
}
}
}
=== modified file 'src/forward.cc'
--- src/forward.cc 2011-01-10 09:43:43 +0000
+++ src/forward.cc 2011-02-10 12:49:24 +0000
@@ -835,7 +835,6 @@
assert(pinned_connection);
fd = pinned_connection->validatePinnedConnection(request, fs->_peer);
if (fd >= 0) {
- pinned_connection->unpinConnection();
#if 0
if (!fs->_peer)
fs->code = HIER_DIRECT;
@@ -851,8 +850,7 @@
return;
}
/* Failure. Fall back on next path */
- debugs(17,2,HERE << " Pinned connection " << pinned_connection << " not valid. Releasing.");
- request->releasePinnedConnection();
+ debugs(17,2,HERE << " Pinned connection " << pinned_connection << " not valid.");
servers = fs->next;
fwdServerFree(fs);
connectStart();
=== modified file 'src/http.cc'
--- src/http.cc 2011-02-07 10:27:53 +0000
+++ src/http.cc 2011-02-10 12:25:37 +0000
@@ -785,7 +785,7 @@
typedef NullaryMemFunT<HttpStateData> CbDialer;
const AsyncCall::Pointer cb = JobCallback(11, 3, CbDialer, this,
HttpStateData::proceedAfter1xx);
- CallJobHere1(11, 4, orig_request->clientConnection, ConnStateData,
+ CallJobHere1(11, 4, orig_request->clientConnectionManager, ConnStateData,
ConnStateData::sendControlMsg, HttpControlMsg(msg, cb));
// If the call is not fired, then the Sink is gone, and HttpStateData
// will terminate due to an aborted store entry or another similar error.
