From 10cfa61fbc5d0720f4e4864f50f1298937327348 Mon Sep 17 00:00:00 2001 From: Mattes D Date: Fri, 23 Jan 2015 23:01:18 +0100 Subject: cNetwork: Added self pointers to keep objects alive for callbacks. Ref.: http://forum.mc-server.org/showthread.php?tid=1700&pid=17947#pid17947 --- src/OSSupport/NetworkSingleton.cpp | 2 ++ src/OSSupport/ServerHandleImpl.cpp | 10 ++++++++-- src/OSSupport/ServerHandleImpl.h | 3 +++ src/OSSupport/TCPLinkImpl.cpp | 25 +++++++++++++++---------- src/OSSupport/TCPLinkImpl.h | 16 +++++++++++----- 5 files changed, 39 insertions(+), 17 deletions(-) diff --git a/src/OSSupport/NetworkSingleton.cpp b/src/OSSupport/NetworkSingleton.cpp index 8c38fb4eb..92f0604cd 100644 --- a/src/OSSupport/NetworkSingleton.cpp +++ b/src/OSSupport/NetworkSingleton.cpp @@ -88,6 +88,8 @@ cNetworkSingleton::~cNetworkSingleton() // Free the underlying LibEvent objects: evdns_base_free(m_DNSBase, true); event_base_free(m_EventBase); + + libevent_global_shutdown(); } diff --git a/src/OSSupport/ServerHandleImpl.cpp b/src/OSSupport/ServerHandleImpl.cpp index 026c0fdc1..ba38dbf2e 100644 --- a/src/OSSupport/ServerHandleImpl.cpp +++ b/src/OSSupport/ServerHandleImpl.cpp @@ -80,6 +80,9 @@ void cServerHandleImpl::Close(void) { conn->Shutdown(); } + + // Remove the ptr to self, so that the object may be freed: + m_SelfPtr.reset(); } @@ -92,6 +95,7 @@ cServerHandleImplPtr cServerHandleImpl::Listen( ) { cServerHandleImplPtr res = cServerHandleImplPtr{new cServerHandleImpl(a_ListenCallbacks)}; + res->m_SelfPtr = res; if (res->Listen(a_Port)) { cNetworkSingleton::Get().AddServer(res); @@ -99,6 +103,7 @@ cServerHandleImplPtr cServerHandleImpl::Listen( else { a_ListenCallbacks->OnError(res->m_ErrorCode, res->m_ErrorMsg); + res->m_SelfPtr.reset(); } return res; } @@ -247,6 +252,7 @@ void cServerHandleImpl::Callback(evconnlistener * a_Listener, evutil_socket_t a_ // Cast to true self: cServerHandleImpl * Self = reinterpret_cast(a_Self); ASSERT(Self != nullptr); + ASSERT(Self->m_SelfPtr != nullptr); // Get the textual IP address and port number out of a_Addr: char IPAddress[128]; @@ -278,13 +284,13 @@ void cServerHandleImpl::Callback(evconnlistener * a_Listener, evutil_socket_t a_ } // Create a new cTCPLink for the incoming connection: - cTCPLinkImplPtr Link = std::make_shared(a_Socket, LinkCallbacks, Self, a_Addr, static_cast(a_Len)); + cTCPLinkImplPtr Link = std::make_shared(a_Socket, LinkCallbacks, Self->m_SelfPtr, a_Addr, static_cast(a_Len)); { cCSLock Lock(Self->m_CS); Self->m_Connections.push_back(Link); } // Lock(m_CS) LinkCallbacks->OnLinkCreated(Link); - Link->Enable(); + Link->Enable(Link); // Call the OnAccepted callback: Self->m_ListenCallbacks->OnAccepted(*Link); diff --git a/src/OSSupport/ServerHandleImpl.h b/src/OSSupport/ServerHandleImpl.h index 33ff787f2..dbb18fc6d 100644 --- a/src/OSSupport/ServerHandleImpl.h +++ b/src/OSSupport/ServerHandleImpl.h @@ -78,6 +78,9 @@ protected: /** Contains the error message for the failure to listen. Only valid for non-listening instances. */ AString m_ErrorMsg; + /** The SharedPtr to self, so that it can be passed to created links. */ + cServerHandleImplPtr m_SelfPtr; + /** Creates a new instance with the specified callbacks. diff --git a/src/OSSupport/TCPLinkImpl.cpp b/src/OSSupport/TCPLinkImpl.cpp index 71b3d572d..b4cefa60c 100644 --- a/src/OSSupport/TCPLinkImpl.cpp +++ b/src/OSSupport/TCPLinkImpl.cpp @@ -17,8 +17,7 @@ cTCPLinkImpl::cTCPLinkImpl(cTCPLink::cCallbacksPtr a_LinkCallbacks): super(a_LinkCallbacks), - m_BufferEvent(bufferevent_socket_new(cNetworkSingleton::Get().GetEventBase(), -1, BEV_OPT_CLOSE_ON_FREE)), - m_Server(nullptr) + m_BufferEvent(bufferevent_socket_new(cNetworkSingleton::Get().GetEventBase(), -1, BEV_OPT_CLOSE_ON_FREE)) { } @@ -26,7 +25,7 @@ cTCPLinkImpl::cTCPLinkImpl(cTCPLink::cCallbacksPtr a_LinkCallbacks): -cTCPLinkImpl::cTCPLinkImpl(evutil_socket_t a_Socket, cTCPLink::cCallbacksPtr a_LinkCallbacks, cServerHandleImpl * a_Server, const sockaddr * a_Address, socklen_t a_AddrLen): +cTCPLinkImpl::cTCPLinkImpl(evutil_socket_t a_Socket, cTCPLink::cCallbacksPtr a_LinkCallbacks, cServerHandleImplPtr a_Server, const sockaddr * a_Address, socklen_t a_AddrLen): super(a_LinkCallbacks), m_BufferEvent(bufferevent_socket_new(cNetworkSingleton::Get().GetEventBase(), a_Socket, BEV_OPT_CLOSE_ON_FREE)), m_Server(a_Server) @@ -59,7 +58,7 @@ cTCPLinkImplPtr cTCPLinkImpl::Connect(const AString & a_Host, UInt16 a_Port, cTC res->m_ConnectCallbacks = a_ConnectCallbacks; cNetworkSingleton::Get().AddLink(res); res->m_Callbacks->OnLinkCreated(res); - res->Enable(); + res->Enable(res); // If a_Host is an IP address, schedule a connection immediately: sockaddr_storage sa; @@ -102,8 +101,11 @@ cTCPLinkImplPtr cTCPLinkImpl::Connect(const AString & a_Host, UInt16 a_Port, cTC -void cTCPLinkImpl::Enable(void) +void cTCPLinkImpl::Enable(cTCPLinkImplPtr a_Self) { + // Take hold of a shared copy of self, to keep as long as the callbacks are coming: + m_Self = a_Self; + // Set the LibEvent callbacks and enable processing: bufferevent_setcb(m_BufferEvent, ReadCallback, nullptr, EventCallback, this); bufferevent_enable(m_BufferEvent, EV_READ | EV_WRITE); @@ -148,6 +150,7 @@ void cTCPLinkImpl::Close(void) { m_Server->RemoveLink(this); } + m_Self.reset(); } @@ -177,7 +180,7 @@ void cTCPLinkImpl::ReadCallback(bufferevent * a_BufferEvent, void * a_Self) void cTCPLinkImpl::EventCallback(bufferevent * a_BufferEvent, short a_What, void * a_Self) { ASSERT(a_Self != nullptr); - cTCPLinkImpl * Self = static_cast(a_Self); + cTCPLinkImplPtr Self = static_cast(a_Self)->m_Self; // If an error is reported, call the error callback: if (a_What & BEV_EVENT_ERROR) @@ -198,13 +201,14 @@ void cTCPLinkImpl::EventCallback(bufferevent * a_BufferEvent, short a_What, void Self->m_Callbacks->OnError(err, evutil_socket_error_to_string(err)); if (Self->m_Server == nullptr) { - cNetworkSingleton::Get().RemoveLink(Self); + cNetworkSingleton::Get().RemoveLink(Self.get()); } else { - Self->m_Server->RemoveLink(Self); + Self->m_Server->RemoveLink(Self.get()); } } + Self->m_Self.reset(); return; } @@ -228,12 +232,13 @@ void cTCPLinkImpl::EventCallback(bufferevent * a_BufferEvent, short a_What, void Self->m_Callbacks->OnRemoteClosed(); if (Self->m_Server != nullptr) { - Self->m_Server->RemoveLink(Self); + Self->m_Server->RemoveLink(Self.get()); } else { - cNetworkSingleton::Get().RemoveLink(Self); + cNetworkSingleton::Get().RemoveLink(Self.get()); } + Self->m_Self.reset(); return; } diff --git a/src/OSSupport/TCPLinkImpl.h b/src/OSSupport/TCPLinkImpl.h index 66347afe0..735e8ed9d 100644 --- a/src/OSSupport/TCPLinkImpl.h +++ b/src/OSSupport/TCPLinkImpl.h @@ -21,6 +21,7 @@ // fwd: class cServerHandleImpl; +typedef SharedPtr cServerHandleImplPtr; class cTCPLinkImpl; typedef SharedPtr cTCPLinkImplPtr; typedef std::vector cTCPLinkImplPtrs; @@ -39,7 +40,7 @@ public: Used for connections accepted in a server using cNetwork::Listen(). a_Address and a_AddrLen describe the remote peer that has connected. The link is created disabled, you need to call Enable() to start the regular communication. */ - cTCPLinkImpl(evutil_socket_t a_Socket, cCallbacksPtr a_LinkCallbacks, cServerHandleImpl * a_Server, const sockaddr * a_Address, socklen_t a_AddrLen); + cTCPLinkImpl(evutil_socket_t a_Socket, cCallbacksPtr a_LinkCallbacks, cServerHandleImplPtr a_Server, const sockaddr * a_Address, socklen_t a_AddrLen); /** Destroys the LibEvent handle representing the link. */ ~cTCPLinkImpl(); @@ -51,8 +52,9 @@ public: /** Enables communication over the link. Links are created with communication disabled, so that creation callbacks can be called first. - This function then enables the regular communication to be reported. */ - void Enable(void); + This function then enables the regular communication to be reported. + The a_Self parameter is used so that the socket can keep itself alive as long as the callbacks are coming. */ + void Enable(cTCPLinkImplPtr a_Self); // cTCPLink overrides: virtual bool Send(const void * a_Data, size_t a_Length) override; @@ -73,8 +75,8 @@ protected: bufferevent * m_BufferEvent; /** The server handle that has created this link. - Only valid for incoming connections, NULL for outgoing connections. */ - cServerHandleImpl * m_Server; + Only valid for incoming connections, nullptr for outgoing connections. */ + cServerHandleImplPtr m_Server; /** The IP address of the local endpoint. Valid only after the socket has been connected. */ AString m_LocalIP; @@ -88,6 +90,10 @@ protected: /** The port of the remote endpoint. Valid only after the socket has been connected. */ UInt16 m_RemotePort; + /** SharedPtr to self, used to keep this object alive as long as the callbacks are coming. + Initialized in Enable(), cleared in Close() and EventCallback(RemoteClosed). */ + cTCPLinkImplPtr m_Self; + /** Creates a new link to be queued to connect to a specified host:port. Used for outgoing connections created using cNetwork::Connect(). -- cgit v1.2.3