From: Christos Tsantilas Date: Wed, 20 Jul 2011 12:38:39 +0000 (+0300) Subject: Author: Christos Tsantilas , Amos Jeffries , Amos Jeffries 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 96c3950362..81dc9b393b 100644 --- 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 25451ef94b..fe4e59f7be 100644 --- 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 ¶ms, 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(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(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 2cd8c079ea..0b600338a0 100644 --- 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 4ac9aac01f..299ffb54b9 100644 --- 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(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(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;