Browse Source

Migrate to shared pointers to controll memory leaks

- Replace QPointers with QSharedPointers to avoid memory leaks in containers
- Delete incomming ptr when QObject* type assigned from outside
Alexey Edelev 6 years ago
parent
commit
6b3d38bfce

+ 5 - 3
src/generator/templates.cpp

@@ -31,7 +31,7 @@ const char *Templates::DefaultProtobufIncludesTemplate = "#include <QMetaType>\n
                                                          "#include <QList>\n"
                                                          "#include <qprotobufobject.h>\n"
                                                          "#include <unordered_map>\n"
-                                                         "#include <QPointer>\n\n";
+                                                         "#include <QSharedPointer>\n\n";
 
 const char *Templates::GlobalEnumClassNameTemplate = "GlobalEnums";
 
@@ -53,9 +53,9 @@ const char *Templates::ComplexTypeRegistrationTemplate = "void $classname$::regi
                                                          "        qRegisterMetaType<$classname$>(\"$namespaces$::$classname$\");\n"
                                                          "        qRegisterMetaType<$classname$List>(\"$namespaces$::$classname$List\");\n"
                                                          "";
-const char *Templates::ComplexListTypeUsingTemplate = "using $classname$List = QList<QPointer<$classname$>>;\n";
+const char *Templates::ComplexListTypeUsingTemplate = "using $classname$List = QList<QSharedPointer<$classname$>>;\n";
 const char *Templates::MapTypeUsingTemplate = "using $classname$ = QMap<$key$, $value$>;\n";
-const char *Templates::MessageMapTypeUsingTemplate = "using $classname$ = QMap<$key$, QPointer<$value$>>;\n";
+const char *Templates::MessageMapTypeUsingTemplate = "using $classname$ = QMap<$key$, QSharedPointer<$value$>>;\n";
 
 const char *Templates::EnumTypeUsingTemplate = "using $enum$List = QList<$enum$>;\n";
 
@@ -110,6 +110,8 @@ const char *Templates::SetterTemplateMessageType = "void set$property_name_cap$_
                                                    "        m_$property_name$ = *$property_name$;\n"
                                                    "        $property_name$Changed();\n"
                                                    "    }\n"
+                                                   "    //NOTE: take ownership of value\n"
+                                                   "    delete $property_name$;\n"
                                                    "}\n\n";
 
 const char *Templates::SetterTemplateComplexType = "void set$property_name_cap$(const $type$ &$property_name$) {\n"

+ 11 - 11
src/protobuf/qprotobufobject_p.h

@@ -26,7 +26,7 @@
 #pragma once
 
 #include <QObject>
-#include <QPointer>
+#include <QSharedPointer>
 #include <QMetaProperty>
 
 #include <unordered_map>
@@ -276,7 +276,7 @@ public:
 
     template<typename V,
              typename std::enable_if_t<std::is_base_of<QObject, V>::value, int> = 0>
-    static QByteArray serializeListType(const QList<QPointer<V>> &listValue, int &outFieldIndex) {
+    static QByteArray serializeListType(const QList<QSharedPointer<V>> &listValue, int &outFieldIndex) {
         qProtoDebug() << __func__ << "listValue.count" << listValue.count() << "outFiledIndex" << outFieldIndex;
 
         if (listValue.count() <= 0) {
@@ -318,8 +318,8 @@ public:
 
     template<typename K, typename V,
              typename std::enable_if_t<std::is_base_of<QObject, V>::value, int> = 0>
-    static QByteArray serializeMap(const QMap<K, QPointer<V>> &mapValue, int &outFieldIndex) {
-        using ItType = typename QMap<K, QPointer<V>>::const_iterator;
+    static QByteArray serializeMap(const QMap<K, QSharedPointer<V>> &mapValue, int &outFieldIndex) {
+        using ItType = typename QMap<K, QSharedPointer<V>>::const_iterator;
         QByteArray mapResult;
         auto kSerializer = serializers[qMetaTypeId<K>()];
         auto vSerializer = serializers[qMetaTypeId<V*>()];
@@ -531,7 +531,7 @@ public:
               typename std::enable_if_t<std::is_base_of<QObject, V>::value, int> = 0>
     static void deserializeMap(QByteArray::const_iterator &it, QVariant &previous) {
         qProtoDebug() << __func__ << "currentByte:" << QString::number((*it), 16);
-        auto out = previous.value<QMap<K, QPointer<V>>>();
+        auto out = previous.value<QMap<K, QSharedPointer<V>>>();
 
         int mapIndex = 0;
         WireTypes type = WireTypes::UnknownWireType;
@@ -552,8 +552,8 @@ public:
             }
         }
 
-        out[key] = value;
-        previous = QVariant::fromValue<QMap<K,QPointer<V>>>(out);
+        out[key] = QSharedPointer<V>(value);
+        previous = QVariant::fromValue<QMap<K,QSharedPointer<V>>>(out);
     }
 
     template <typename T,
@@ -578,7 +578,7 @@ public:
     template<typename T>
     static void registerSerializers() {
         ProtobufObjectPrivate::wrapSerializer<T>(serializeComplexType<T>, deserializeComplexType<T>, LengthDelimited);
-        ProtobufObjectPrivate::serializers[qMetaTypeId<QList<QPointer<T>>>()] = {ProtobufObjectPrivate::Serializer(serializeComplexListType<T>),
+        ProtobufObjectPrivate::serializers[qMetaTypeId<QList<QSharedPointer<T>>>()] = {ProtobufObjectPrivate::Serializer(serializeComplexListType<T>),
                 ProtobufObjectPrivate::Deserializer(deserializeComplexListType<T>), LengthDelimited};
     }
 
@@ -599,16 +599,16 @@ public:
     template <typename T,
               typename std::enable_if_t<std::is_base_of<QObject, T>::value, int> = 0>
     static QByteArray serializeComplexListType(const QVariant &listValue, int &outFieldIndex) {
-        QList<QPointer<T>> list = listValue.value<QList<QPointer<T>>>();
+        QList<QSharedPointer<T>> list = listValue.value<QList<QSharedPointer<T>>>();
         return ProtobufObjectPrivate::serializeListType(list, outFieldIndex);
     }
 
     template <typename T,
               typename std::enable_if_t<std::is_base_of<QObject, T>::value, int> = 0>
     static void deserializeComplexListType(QByteArray::const_iterator &it, QVariant &previous) {
-        QList<QPointer<T>> previousList = previous.value<QList<QPointer<T>>>();
+        QList<QSharedPointer<T>> previousList = previous.value<QList<QSharedPointer<T>>>();
         QVariant newMember = ProtobufObjectPrivate::deserializeList<T>(it);
-        previousList.append(newMember.value<T*>());
+        previousList.append(QSharedPointer<T>(newMember.value<T*>()));
         previous.setValue(previousList);
     }
 

+ 11 - 11
tests/serializationcomplexmessagemap.cpp

@@ -47,7 +47,7 @@ using namespace qtprotobuf;
 TEST_F(SerializationTest, SimpleFixed32ComplexMapSerializeTest)
 {
     SimpleFixed32ComplexMessageMapMessage test;
-    test.setMapField({{10, new ComplexMessage{16, {"ten sixteen"}}}, {42, new ComplexMessage{10, {"fourty two ten sixteen"}}}, {65555, new ComplexMessage{10, {"WUT?"}}}});
+    test.setMapField({{10, QSharedPointer<ComplexMessage>(new ComplexMessage{16, {"ten sixteen"}})}, {42, QSharedPointer<ComplexMessage>(new ComplexMessage{10, {"fourty two ten sixteen"}})}, {65555, QSharedPointer<ComplexMessage>(new ComplexMessage{10, {"WUT?"}})}});
     QByteArray result = test.serialize();
 
     ASSERT_STREQ(result.toHex().toStdString().c_str(),
@@ -57,7 +57,7 @@ TEST_F(SerializationTest, SimpleFixed32ComplexMapSerializeTest)
 TEST_F(SerializationTest, SimpleSFixed32ComplexMapSerializeTest)
 {
     SimpleSFixed32ComplexMessageMapMessage test;
-    test.setMapField({{10, new ComplexMessage{16 , {"ten sixteen"}}}, {-42, new ComplexMessage{10 , {"minus fourty two ten sixteen"}}}, {65555, new ComplexMessage{10 , {"WUT?"}}}});
+    test.setMapField({{10, QSharedPointer<ComplexMessage>(new ComplexMessage{16 , {"ten sixteen"}})}, {-42, QSharedPointer<ComplexMessage>(new ComplexMessage{10 , {"minus fourty two ten sixteen"}})}, {65555, QSharedPointer<ComplexMessage>(new ComplexMessage{10 , {"WUT?"}})}});
     QByteArray result = test.serialize();
 
     ASSERT_STREQ(result.toHex().toStdString().c_str(),
@@ -67,7 +67,7 @@ TEST_F(SerializationTest, SimpleSFixed32ComplexMapSerializeTest)
 TEST_F(SerializationTest, SimpleInt32ComplexMapSerializeTest)
 {
     SimpleInt32ComplexMessageMapMessage test;
-    test.setMapField({{10, new ComplexMessage{16 , {"ten sixteen"}}}, {-42, new ComplexMessage{10 , {"minus fourty two ten sixteen"}}}, {65555, new ComplexMessage{10 , {"WUT?"}}}});
+    test.setMapField({{10, QSharedPointer<ComplexMessage>(new ComplexMessage{16 , {"ten sixteen"}})}, {-42, QSharedPointer<ComplexMessage>(new ComplexMessage{10 , {"minus fourty two ten sixteen"}})}, {65555, QSharedPointer<ComplexMessage>(new ComplexMessage{10 , {"WUT?"}})}});
     QByteArray result = test.serialize();
 
     ASSERT_STREQ(result.toHex().toStdString().c_str(),
@@ -77,7 +77,7 @@ TEST_F(SerializationTest, SimpleInt32ComplexMapSerializeTest)
 TEST_F(SerializationTest, SimpleSInt32ComplexMapSerializeTest)
 {
     SimpleSInt32ComplexMessageMapMessage test;
-    test.setMapField({{10, new ComplexMessage{16 , {"ten sixteen"}}}, {42, new ComplexMessage{10 , {"fourty two ten sixteen"}}}, {-65555, new ComplexMessage{10 , {"minus WUT?"}}}});
+    test.setMapField({{10, QSharedPointer<ComplexMessage>(new ComplexMessage{16 , {"ten sixteen"}})}, {42, QSharedPointer<ComplexMessage>(new ComplexMessage{10 , {"fourty two ten sixteen"}})}, {-65555, QSharedPointer<ComplexMessage>(new ComplexMessage{10 , {"minus WUT?"}})}});
     QByteArray result = test.serialize();
 
     ASSERT_STREQ(result.toHex().toStdString().c_str(),
@@ -87,7 +87,7 @@ TEST_F(SerializationTest, SimpleSInt32ComplexMapSerializeTest)
 TEST_F(SerializationTest, SimpleUInt32ComplexMapSerializeTest)
 {
     SimpleUInt32ComplexMessageMapMessage test;
-    test.setMapField({{10, new ComplexMessage{16 , {"ten sixteen"}}}, {42, new ComplexMessage{10 , {"fourty two ten sixteen"}}}, {65555, new ComplexMessage{10 , {"WUT?"}}}});
+    test.setMapField({{10, QSharedPointer<ComplexMessage>(new ComplexMessage{16 , {"ten sixteen"}})}, {42, QSharedPointer<ComplexMessage>(new ComplexMessage{10 , {"fourty two ten sixteen"}})}, {65555, QSharedPointer<ComplexMessage>(new ComplexMessage{10 , {"WUT?"}})}});
     QByteArray result = test.serialize();
 
     ASSERT_STREQ(result.toHex().toStdString().c_str(),
@@ -97,7 +97,7 @@ TEST_F(SerializationTest, SimpleUInt32ComplexMapSerializeTest)
 TEST_F(SerializationTest, SimpleFixed64ComplexMapSerializeTest)
 {
     SimpleFixed64ComplexMessageMapMessage test;
-    test.setMapField({{10, new ComplexMessage{16 , {"ten sixteen"}}}, {UINT64_MAX, new ComplexMessage{42 , {"minus fourty two ten MAAAX"}}}, {65555, new ComplexMessage{10 , {"WUT?"}}}});
+    test.setMapField({{10, QSharedPointer<ComplexMessage>(new ComplexMessage{16 , {"ten sixteen"}})}, {UINT64_MAX, QSharedPointer<ComplexMessage>(new ComplexMessage{42 , {"minus fourty two ten MAAAX"}})}, {65555, QSharedPointer<ComplexMessage>(new ComplexMessage{10 , {"WUT?"}})}});
     QByteArray result = test.serialize();
 
     ASSERT_STREQ(result.toHex().toStdString().c_str(),
@@ -107,7 +107,7 @@ TEST_F(SerializationTest, SimpleFixed64ComplexMapSerializeTest)
 TEST_F(SerializationTest, SimpleSFixed64ComplexMapSerializeTest)
 {
     SimpleSFixed64ComplexMessageMapMessage test;
-    test.setMapField({{10, new ComplexMessage{16 , {"ten sixteen"}}}, {-42, new ComplexMessage{10 , {"minus fourty two ten sixteen"}}}, {65555, new ComplexMessage{10 , {"WUT?"}}}});
+    test.setMapField({{10, QSharedPointer<ComplexMessage>(new ComplexMessage{16 , {"ten sixteen"}})}, {-42, QSharedPointer<ComplexMessage>(new ComplexMessage{10 , {"minus fourty two ten sixteen"}})}, {65555, QSharedPointer<ComplexMessage>(new ComplexMessage{10 , {"WUT?"}})}});
     QByteArray result = test.serialize();
 
     ASSERT_STREQ(result.toHex().toStdString().c_str(),
@@ -117,7 +117,7 @@ TEST_F(SerializationTest, SimpleSFixed64ComplexMapSerializeTest)
 TEST_F(SerializationTest, SimpleInt64ComplexMapSerializeTest)
 {
     SimpleInt64ComplexMessageMapMessage test;
-    test.setMapField({{10, new ComplexMessage{16 , {"ten sixteen"}}}, {-42, new ComplexMessage{10 , {"minus fourty two ten sixteen"}}}, {65555, new ComplexMessage{10 , {"WUT?"}}}});
+    test.setMapField({{10, QSharedPointer<ComplexMessage>(new ComplexMessage{16 , {"ten sixteen"}})}, {-42, QSharedPointer<ComplexMessage>(new ComplexMessage{10 , {"minus fourty two ten sixteen"}})}, {65555, QSharedPointer<ComplexMessage>(new ComplexMessage{10 , {"WUT?"}})}});
     QByteArray result = test.serialize();
 
     ASSERT_STREQ(result.toHex().toStdString().c_str(),
@@ -127,7 +127,7 @@ TEST_F(SerializationTest, SimpleInt64ComplexMapSerializeTest)
 TEST_F(SerializationTest, SimpleSInt64ComplexMapSerializeTest)
 {
     SimpleSInt64ComplexMessageMapMessage test;
-    test.setMapField({{10, new ComplexMessage{16 , {"ten sixteen"}}}, {-42, new ComplexMessage{10 , {"minus fourty two ten sixteen"}}}, {65555, new ComplexMessage{10 , {"WUT?"}}}});
+    test.setMapField({{10, QSharedPointer<ComplexMessage>(new ComplexMessage{16 , {"ten sixteen"}})}, {-42, QSharedPointer<ComplexMessage>(new ComplexMessage{10 , {"minus fourty two ten sixteen"}})}, {65555, QSharedPointer<ComplexMessage>(new ComplexMessage{10 , {"WUT?"}})}});
     QByteArray result = test.serialize();
     ASSERT_STREQ(result.toHex().toStdString().c_str(),
                 "122608531222121e321c6d696e757320666f757274792074776f2074656e207369787465656e080a121508141211120d320b74656e207369787465656e0810121008a68008120a120632045755543f080a");
@@ -136,7 +136,7 @@ TEST_F(SerializationTest, SimpleSInt64ComplexMapSerializeTest)
 TEST_F(SerializationTest, SimpleUInt64ComplexMapSerializeTest)
 {
     SimpleUInt64ComplexMessageMapMessage test;
-    test.setMapField({{10, new ComplexMessage{11 , {"ten eleven"}}}, {42, new ComplexMessage{10 , {"fourty two ten sixteen"}}}, {65555, new ComplexMessage{10 , {"WUT?"}}}});
+    test.setMapField({{10, QSharedPointer<ComplexMessage>(new ComplexMessage{11 , {"ten eleven"}})}, {42, QSharedPointer<ComplexMessage>(new ComplexMessage{10 , {"fourty two ten sixteen"}})}, {65555, QSharedPointer<ComplexMessage>(new ComplexMessage{10 , {"WUT?"}})}});
     QByteArray result = test.serialize();
 
     ASSERT_STREQ(result.toHex().toStdString().c_str(),
@@ -146,7 +146,7 @@ TEST_F(SerializationTest, SimpleUInt64ComplexMapSerializeTest)
 TEST_F(SerializationTest, SimpleStringComplexMapSerializeTest)
 {
     SimpleStringComplexMessageMapMessage test;
-    test.setMapField({{"ben", new ComplexMessage{11 , {"ten eleven"}}}, {"where is my car dude?", new ComplexMessage{10 , {"fourty two ten sixteen"}}}, {"WUT??", new ComplexMessage{10 , {"?WUT?"}}}});
+    test.setMapField({{"ben", QSharedPointer<ComplexMessage>(new ComplexMessage{11 , {"ten eleven"}})}, {"where is my car dude?", QSharedPointer<ComplexMessage>(new ComplexMessage{10 , {"fourty two ten sixteen"}})}, {"WUT??", QSharedPointer<ComplexMessage>(new ComplexMessage{10 , {"?WUT?"}})}});
     QByteArray result = test.serialize();
 
     ASSERT_STREQ(result.toHex().toStdString().c_str(),

+ 4 - 4
tests/serializationtest.cpp

@@ -1523,11 +1523,11 @@ TEST_F(SerializationTest, RepeatedComplexMessageTest)
 {
     SimpleStringMessage stringMsg;
     stringMsg.setTestFieldString("qwerty");
-    ComplexMessage msg;
-    msg.setTestFieldInt(25);
-    msg.setTestComplexField(stringMsg);
+    QSharedPointer<ComplexMessage> msg(new ComplexMessage);
+    msg->setTestFieldInt(25);
+    msg->setTestComplexField(stringMsg);
     RepeatedComplexMessage test;
-    test.setTestRepeatedComplex({&msg, &msg, &msg});
+    test.setTestRepeatedComplex({msg, msg, msg});
     QByteArray result = test.serialize();
     //qDebug() << "result " << result.toHex();
 

+ 3 - 3
tests/simpletest.cpp

@@ -403,11 +403,11 @@ TEST_F(SimpleTest, RepeatedExternalComplexMessageTest)
     qtprotobufnamespace1::externaltests::SimpleExternalMessage complexMessage;
     complexMessage.setLocalList({1, 2, 3, 4, 5});
 
-    qtprotobufnamespace1::externaltests::ExternalComplexMessage externalMessage;
-    externalMessage.setTestFieldInt(complexMessage);
+    QSharedPointer<qtprotobufnamespace1::externaltests::ExternalComplexMessage> externalMessage(new qtprotobufnamespace1::externaltests::ExternalComplexMessage);
+    externalMessage->setTestFieldInt(complexMessage);
 
     qtprotobufnamespace1::externaltests::ExternalComplexMessageList complexMessageList;
-    complexMessageList << &externalMessage;
+    complexMessageList << externalMessage;
 
     ASSERT_TRUE(test.setProperty(propertyName, QVariant::fromValue(complexMessageList)));
     ASSERT_TRUE(test.property(propertyName).value<qtprotobufnamespace1::externaltests::ExternalComplexMessageList>() == complexMessageList);