Bugfix: Use 'smart' pointer to refcount and auto-destruct to avoid notification memory leaks

Updates issue 144

rpc/rpcconn.cpp

@@ -195,7 +195,7 @@ _exit:
 }
 
 void RpcConnection::sendNotification(int notifType,
-        ::google::protobuf::Message *notifData)
+        SharedProtobufMessage notifData)
 {
     char msgBuf[PB_HDR_SIZE];
     char* const msg = &msgBuf[0];

rpc/rpcconn.h

@@ -20,6 +20,8 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>
 #ifndef _RPC_CONNECTION_H
 #define _RPC_CONNECTION_H
 
+#include "sharedprotobufmessage.h"
+
 #include <QAbstractSocket>
 
 // forward declarations
@@ -55,7 +57,7 @@ signals:
     void closed();
 
 public slots:
-    void sendNotification(int notifType, ::google::protobuf::Message *notifData);
+    void sendNotification(int notifType, SharedProtobufMessage notifData);
 
 private slots:
     void start();

rpc/rpcserver.cpp

@@ -76,8 +76,8 @@ void RpcServer::incomingConnection(int socketDescriptor)
     // setup thread to "self-destruct" when it is done
     connect(thread, SIGNAL(finished()), thread, SLOT(deleteLater()));
 
-    connect(this, SIGNAL(notifyClients(int, ::google::protobuf::Message*)),
+    connect(this, SIGNAL(notifyClients(int, SharedProtobufMessage)),
-            conn, SLOT(sendNotification(int, ::google::protobuf::Message*)));
+            conn, SLOT(sendNotification(int, SharedProtobufMessage)));
 
     thread->start();
 }

rpc/rpcserver.h

@@ -20,6 +20,8 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>
 #ifndef _RPC_SERVER_H
 #define _RPC_SERVER_H
 
+#include "sharedprotobufmessage.h"
+
 #include <QTcpServer>
 
 // forward declaration
@@ -42,7 +44,7 @@ public:
         quint16 tcpPortNum);
 
 signals:
-    void notifyClients(int notifType, ::google::protobuf::Message *notifData);
+    void notifyClients(int notifType, SharedProtobufMessage notifData);
 
 protected:
     void incomingConnection(int socketDescriptor);

rpc/sharedprotobufmessage.h

@@ -0,0 +1,88 @@
+/*
+Copyright (C) 2015 Srivats P.
+
+This file is part of "Ostinato"
+
+This is free software: you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation, either version 3 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program.  If not, see <http://www.gnu.org/licenses/>
+*/
+
+#ifndef _SHARED_PROTOBUF_MESSAGE_H
+#define _SHARED_PROTOBUF_MESSAGE_H
+
+#include <google/protobuf/message.h>
+#include <QMutex>
+
+// TODO: Use QSharedPointer instead once the minimum Qt version becomes >= 4.5
+
+template <class T>
+class SharedPointer
+{
+public:
+    SharedPointer(T *ptr = 0)
+        : ptr_(ptr)
+    {
+        mutex_ = new QMutex();
+        refCnt_ = new unsigned int;
+        *refCnt_ = 1;
+        qDebug("sharedptr %p(constr) refcnt %p(%u)", this, refCnt_, *refCnt_);
+    }
+
+    ~SharedPointer()
+    {
+        mutex_->lock();
+
+        (*refCnt_)--;
+        if (*refCnt_ == 0) {
+            delete ptr_;
+            delete refCnt_;
+
+            mutex_->unlock();
+            delete mutex_;
+            qDebug("sharedptr %p destroyed", this);
+            return;
+        }
+
+        qDebug("sharedptr %p(destr) refcnt %p(%u)", this, refCnt_, *refCnt_);
+        mutex_->unlock();
+    }
+
+    SharedPointer(const SharedPointer<T> &other)
+    {
+        ptr_ = other.ptr_;
+        refCnt_ = other.refCnt_;
+        mutex_ = other.mutex_;
+
+        mutex_->lock();
+        (*refCnt_)++;
+        qDebug("sharedptr %p(copy) refcnt %p(%u)", this, refCnt_,*refCnt_);
+        mutex_->unlock();
+    }
+
+    T* operator->() const
+    {
+        return ptr_;
+    }
+
+protected:
+    T *ptr_;
+
+    // use uint+mutex to simulate a QAtomicInt
+    unsigned int *refCnt_;
+    QMutex *mutex_;
+};
+
+typedef class SharedPointer< ::google::protobuf::Message> SharedProtobufMessage;
+
+#endif
+

server/drone.cpp

@@ -22,6 +22,8 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>
 #include "rpcserver.h"
 #include "myservice.h"
 
+#include <QMetaType>
+
 extern int myport;
 extern const char* version;
 extern const char* revision;
@@ -43,14 +45,16 @@ bool Drone::init()
 {
     Q_ASSERT(rpcServer);
 
+    qRegisterMetaType<SharedProtobufMessage>("SharedProtobufMessage");
+
     if (!rpcServer->registerService(service, myport ? myport : 7878))
     {
         //qCritical(qPrintable(rpcServer->errorString()));
         return false;
     }
 
-    connect(service, SIGNAL(notification(int, ::google::protobuf::Message*)), 
+    connect(service, SIGNAL(notification(int, SharedProtobufMessage)), 
-            rpcServer, SIGNAL(notifyClients(int, ::google::protobuf::Message*)));
+            rpcServer, SIGNAL(notifyClients(int, SharedProtobufMessage)));
 
     return true;
 }

server/myservice.cpp

@@ -137,8 +137,7 @@ void MyService::modifyPort(::google::protobuf::RpcController* /*controller*/,
 
     if (notif->port_id_list().port_id_size()) {
         notif->set_notif_type(OstProto::portConfigChanged);
-        emit notification(notif->notif_type(), notif);
+        emit notification(notif->notif_type(), SharedProtobufMessage(notif));
-        // FIXME: who will free notif!
     }
 }
 

server/myservice.h

@@ -21,6 +21,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>
 #define _MY_SERVICE_H
 
 #include "../common/protocol.pb.h"
+#include "../rpc/sharedprotobufmessage.h"
 
 #include <QList>
 #include <QObject>
@@ -105,7 +106,7 @@ public:
         ::google::protobuf::Closure* done);
 
 signals:
-    void notification(int notifType, ::google::protobuf::Message *notifData);
+    void notification(int notifType, SharedProtobufMessage notifData);
 
 private:
     /*