Author: Christos Tsantilas <chtsanti@users.sourceforge.net>, Amos Jeffries <squid3...

Bug 3264: Segmentation Fault in src/ipc/Strand.cc(54) receive: 3

The SharedListenResponse because copied using memcpy function (TypedMsgHdr.cc
file, 154 line, Ipc::TypedMsgHdr::getRaw method) can not have complex class
members like the SharedListenResponse::conn which is a RefCounted object.

This patch
 - Remove the SharedListenResponse::conn member and replaced with a
   single SharedListenResponse::fd (integer filedescriptor) member.

 - Does not create a new Comm::Connection object for listening sockets
   inside IPC code , but use the Comm:Connection object created while
   initializing the listening socket and passed to the Ipc::StartListening
   method.

diff --git a/src/ipc/Coordinator.cc b/src/ipc/Coordinator.cc
index 96c3950362c5605a8d2431651291a9c7db8b3bcb..81dc9b393b66db84a4eb89b7d0df273233780047 100644 (file)
--- a/src/ipc/Coordinator.cc
+++ b/src/ipc/Coordinator.cc
@@ -128,7 +128,7 @@ Ipc::Coordinator::handleSharedListenRequest(const SharedListenRequest& request)
            request.params.addr << " to kid" << request.requestorId <<
            " mapId=" << request.mapId);
 
-    SharedListenResponse response(c, errNo, request.mapId);
+    SharedListenResponse response(c->fd, errNo, request.mapId);
     TypedMsgHdr message;
     response.pack(message);
     SendMessage(MakeAddr(strandAddrPfx, request.requestorId), message);

diff --git a/src/ipc/SharedListen.cc b/src/ipc/SharedListen.cc
index 25451ef94b07962b91bf24e8a83abee680ab4afe..fe4e59f7be3f19517020906d0178d615bcb5be3a 100644 (file)
--- a/src/ipc/SharedListen.cc
+++ b/src/ipc/SharedListen.cc
@@ -82,18 +82,17 @@ void Ipc::SharedListenRequest::pack(TypedMsgHdr &hdrMsg) const
 }
 
 
-Ipc::SharedListenResponse::SharedListenResponse(const Comm::ConnectionPointer &c, int anErrNo, int aMapId):
-        conn(c), errNo(anErrNo), mapId(aMapId)
+Ipc::SharedListenResponse::SharedListenResponse(int aFd, int anErrNo, int aMapId):
+        fd(aFd), errNo(anErrNo), mapId(aMapId)
 {
 }
 
 Ipc::SharedListenResponse::SharedListenResponse(const TypedMsgHdr &hdrMsg):
-        conn(NULL), errNo(0), mapId(-1)
+    fd(-1), errNo(0), mapId(-1)
 {
     hdrMsg.checkType(mtSharedListenResponse);
     hdrMsg.getPod(*this);
-    conn = new Comm::Connection;
-    conn->fd = hdrMsg.getFd();
+    fd = hdrMsg.getFd();
     // other conn details are passed in OpenListenerParams and filled out by SharedListenJoin()
 }
 
@@ -101,7 +100,7 @@ void Ipc::SharedListenResponse::pack(TypedMsgHdr &hdrMsg) const
 {
     hdrMsg.setType(mtSharedListenResponse);
     hdrMsg.putPod(*this);
-    hdrMsg.putFd(conn->fd);
+    hdrMsg.putFd(fd);
 }
 
 
@@ -127,10 +126,8 @@ void Ipc::JoinSharedListen(const OpenListenerParams &params,
 
 void Ipc::SharedListenJoined(const SharedListenResponse &response)
 {
-    Comm::ConnectionPointer c = response.conn;
-
     // Dont debugs c fully since only FD is filled right now.
-    debugs(54, 3, HERE << "got listening FD " << c->fd << " errNo=" <<
+    debugs(54, 3, HERE << "got listening FD " << response.fd << " errNo=" <<
            response.errNo << " mapId=" << response.mapId);
 
     Must(TheSharedListenRequestMap.find(response.mapId) != TheSharedListenRequestMap.end());
@@ -138,22 +135,24 @@ void Ipc::SharedListenJoined(const SharedListenResponse &response)
     Must(por.callback != NULL);
     TheSharedListenRequestMap.erase(response.mapId);
 
-    if (Comm::IsConnOpen(c)) {
+    StartListeningCb *cbd = dynamic_cast<StartListeningCb*>(por.callback->getDialer());
+    assert(cbd && cbd->conn != NULL);
+    Must(cbd && cbd->conn != NULL);
+    cbd->conn->fd = response.fd;
+
+    if (Comm::IsConnOpen(cbd->conn)) {
         OpenListenerParams &p = por.params;
-        c->local = p.addr;
-        c->flags = p.flags;
+        cbd->conn->local = p.addr;
+        cbd->conn->flags = p.flags;
         // XXX: leave the comm AI stuff to comm_import_opened()?
         struct addrinfo *AI = NULL;
         p.addr.GetAddrInfo(AI);
         AI->ai_socktype = p.sock_type;
         AI->ai_protocol = p.proto;
-        comm_import_opened(c, FdNote(p.fdNote), AI);
+        comm_import_opened(cbd->conn, FdNote(p.fdNote), AI);
         p.addr.FreeAddrInfo(AI);
     }
 
-    StartListeningCb *cbd = dynamic_cast<StartListeningCb*>(por.callback->getDialer());
-    Must(cbd);
-    cbd->conn = c;
     cbd->errNo = response.errNo;
     cbd->handlerSubscription = por.params.handlerSubscription;
     ScheduleCallHere(por.callback);

diff --git a/src/ipc/SharedListen.h b/src/ipc/SharedListen.h
index 2cd8c079ea57b22584026ffacbfabe9b4715d9fa..0b600338a086bd3f138d57d8572b0d09ca88826c 100644 (file)
--- a/src/ipc/SharedListen.h
+++ b/src/ipc/SharedListen.h
@@ -60,12 +60,12 @@ public:
 class SharedListenResponse
 {
 public:
-    SharedListenResponse(const Comm::ConnectionPointer &c, int errNo, int mapId);
+    SharedListenResponse(int fd, int errNo, int mapId);
     explicit SharedListenResponse(const TypedMsgHdr &hdrMsg); ///< from recvmsg()
     void pack(TypedMsgHdr &hdrMsg) const; ///< prepare for sendmsg()
 
 public:
-    Comm::ConnectionPointer conn; ///< opened listening socket or -1
+    int fd; ///< opened listening socket or -1
     int errNo; ///< errno value from comm_open_sharedListen() call
     int mapId; ///< to map future response to the requestor's callback
 };

diff --git a/src/ipc/StartListening.cc b/src/ipc/StartListening.cc
index 4ac9aac01f12ef7ceabb80514ae1045c5867dafa..299ffb54b938f6d6c5b479e927abfbeecd0e13e4 100644 (file)
--- a/src/ipc/StartListening.cc
+++ b/src/ipc/StartListening.cc
@@ -30,6 +30,10 @@ void
 Ipc::StartListening(int sock_type, int proto, const Comm::ConnectionPointer &listenConn,
                     FdNoteId fdNote, AsyncCall::Pointer &callback)
 {
+    StartListeningCb *cbd = dynamic_cast<StartListeningCb*>(callback->getDialer());
+    Must(cbd);
+    cbd->conn = listenConn;
+
     if (UsingSmp()) { // if SMP is on, share
         OpenListenerParams p;
         p.sock_type = sock_type;
@@ -41,10 +45,6 @@ Ipc::StartListening(int sock_type, int proto, const Comm::ConnectionPointer &lis
         return; // wait for the call back
     }
 
-    StartListeningCb *cbd = dynamic_cast<StartListeningCb*>(callback->getDialer());
-    Must(cbd);
-    cbd->conn = listenConn;
-
     enter_suid();
     comm_open_listener(sock_type, proto, cbd->conn, FdNote(fdNote));
     cbd->errNo = Comm::IsConnOpen(cbd->conn) ? 0 : errno;