Commit 9515cfd6 authored by luz's avatar luz

Fixed Memory leak: automatic callback clear in SocketComm and subclasses when...

Fixed Memory leak: automatic callback clear in SocketComm and subclasses when connection closes by itself now breaks retain cycles caused by callbacks that hold a smart ptr to the connection
parent 9580265b
......@@ -124,6 +124,9 @@ namespace p44 {
/// @param aFd optional; fd to switch to non-blocking, defaults to this FdConn's fd set with setFd()
void makeNonBlocking(int aFd = -1);
/// clear all callbacks
/// @note this is important because handlers might cause retain cycles when they have smart ptr arguments
virtual void clearCallbacks() { receiveHandler = NULL; transmitHandler = NULL; }
protected:
/// this is intended to be overridden in subclases, and is called when
......@@ -138,6 +141,8 @@ namespace p44 {
class FdStringCollector : public FdComm
{
typedef FdComm inherited;
bool ended; ///< set when FD returns error or HUP
FdCommCB endedCallback; ///< called when collecting ends (after setup by collectToEnd())
......@@ -150,6 +155,10 @@ namespace p44 {
/// collect until file descriptor does not provide any more data
void collectToEnd(FdCommCB aEndedCallback);
/// clear all callbacks
/// @note this is important because handlers might cause retain cycles when they have smart ptr arguments
virtual void clearCallbacks() { endedCallback = NULL; inherited::clearCallbacks(); }
protected:
virtual void dataExceptionHandler(int aFd, int aPollFlags);
......
......@@ -74,6 +74,10 @@ namespace p44 {
/// request closing connection after last message has been sent
void closeAfterSend();
/// clear all callbacks
/// @note this is important because handlers might cause retain cycles when they have smart ptr arguments
virtual void clearCallbacks() { jsonMessageHandler = NULL; inherited::clearCallbacks(); }
private:
void gotData(ErrorPtr aError);
......
......@@ -125,6 +125,9 @@ namespace p44 {
/// @result empty or Error object in case of error sending error response
ErrorPtr sendError(const char *aJsonRpcId, ErrorPtr aErrorToSend);
/// clear all callbacks
/// @note this is important because handlers might cause retain cycles when they have smart ptr arguments
virtual void clearCallbacks() { jsonRequestHandler = NULL; inherited::clearCallbacks(); }
private:
void gotJson(ErrorPtr aError, JsonObjectPtr aJsonObject);
......
......@@ -32,6 +32,7 @@ SocketComm::SocketComm(MainLoop &aMainLoop) :
isConnecting(false),
isClosing(false),
serving(false),
clearHandlersAtClose(false),
addressInfoList(NULL),
currentAddressInfo(NULL),
currentSockAddrP(NULL),
......@@ -498,7 +499,9 @@ void SocketComm::internalCloseConnection()
serving = false;
// - close all child connections (closing will remove them from the list)
while (clientConnections.size()>0) {
(*clientConnections.begin())->closeConnection();
SocketCommPtr conn = *clientConnections.begin();
conn->closeConnection();
conn->clearCallbacks(); // clear callbacks to break possible retain cycles
}
}
else if (connectionOpen || isConnecting) {
......@@ -524,6 +527,10 @@ void SocketComm::internalCloseConnection()
free(currentSockAddrP);
currentSockAddrP = NULL;
}
// now clear handlers if requested
if (clearHandlersAtClose) {
clearCallbacks();
}
isClosing = false;
}
......
......@@ -96,6 +96,7 @@ namespace p44 {
bool isClosing; ///< in progress of closing connection
bool connectionOpen; ///< regular data connection is open
bool serving; ///< is serving socket
bool clearHandlersAtClose; ///< when socket closes, all handlers are cleared (to break retain cycles)
SocketCommCB connectionStatusHandler;
// server connection internals
int maxServerConnections;
......@@ -178,6 +179,14 @@ namespace p44 {
/// @note for UDP, the host/port specified in setConnectionParams() will be used to send datagrams to
virtual size_t transmitBytes(size_t aNumBytes, const uint8_t *aBytes, ErrorPtr &aError);
/// clear all callbacks
/// @note this is important because handlers might cause retain cycles when they have smart ptr arguments
virtual void clearCallbacks() { connectionStatusHandler = NULL; serverConnectionHandler = NULL; inherited::clearCallbacks(); }
/// make sure handlers are cleared as soon as connection closes
/// @note this is for connections that only live by themselves and should deallocate when they close. As handlers might hold
/// smart pointers to the connection, it is essential the handlers are cleared
void setClearHandlersAtClose() { clearHandlersAtClose = true; }
private:
void freeAddressInfo();
......
......@@ -89,10 +89,12 @@ namespace p44 {
/// @param aTargetMustMatch if set, only direct responses to our search are returned
void startSearchForTarget(SsdpSearchCB aSearchResultHandler, const char *aSearchTarget, bool aSingleTarget, bool aTargetMustMatch);
/// stop SSDP search - result handler
void stopSearch();
/// clear all callbacks
/// @note this is important because handlers might cause retain cycles when they have smart ptr arguments
virtual void clearCallbacks() { searchResultHandler = NULL; inherited::clearCallbacks(); }
private:
void gotData(ErrorPtr aError);
......
......@@ -214,6 +214,7 @@ SocketCommPtr P44VdcHost::configApiConnectionHandler(SocketCommPtr aServerSocket
{
JsonCommPtr conn = JsonCommPtr(new JsonComm(MainLoop::currentMainLoop()));
conn->setMessageHandler(boost::bind(&P44VdcHost::configApiRequestHandler, this, conn, _1, _2));
conn->setClearHandlersAtClose(); // close must break retain cycles so this object won't cause a mem leak
return conn;
}
......
......@@ -38,6 +38,13 @@ void VdcApiServer::start()
}
void VdcApiServer::stop()
{
closeConnection();
clearCallbacks();
}
void VdcApiServer::setConnectionStatusHandler(VdcApiConnectionCB aConnectionCB)
{
apiConnectionStatusHandler = aConnectionCB;
......@@ -49,6 +56,7 @@ SocketCommPtr VdcApiServer::serverConnectionHandler(SocketCommPtr aServerSocketC
// create new connection
VdcApiConnectionPtr apiConnection = newConnection();
SocketCommPtr socketComm = apiConnection->socketConnection();
socketComm->setClearHandlersAtClose(); // to make sure retain cycles are broken
socketComm->relatedObject = apiConnection; // bind object to connection
socketComm->setConnectionStatusHandler(boost::bind(&VdcApiServer::connectionStatusHandler, this, _1, _2));
// return the socketComm object which handles this connection
......@@ -86,6 +94,7 @@ void VdcApiConnection::closeConnection()
{
if (socketConnection()) {
socketConnection()->closeConnection();
socketConnection()->clearCallbacks();
}
}
......
......@@ -143,6 +143,10 @@ namespace p44 {
/// stop API server, close all connections
void stop();
/// clear all callbacks
/// @note this is important because handlers might cause retain cycles when they have smart ptr arguments
virtual void clearCallbacks() { apiConnectionStatusHandler = NULL; inherited::clearCallbacks(); }
protected:
/// create API connection of correct type for this API server
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment