Thread safety implemented for registerPacker and registerUnpacker

src/private/pack_p.cpp

@@ -11,6 +11,7 @@
 
 QHash<QMetaType::Type, MsgPackPrivate::packer_t> MsgPackPrivate::user_packers;
 bool MsgPackPrivate::compatibilityMode = false;
+QReadWriteLock MsgPackPrivate::packers_lock;
 
 quint8 *MsgPackPrivate::pack(const QVariant &v, quint8 *p, bool wr)
 {
@@ -38,7 +39,12 @@ quint8 *MsgPackPrivate::pack(const QVariant &v, quint8 *p, bool wr)
     else if (t == QMetaType::QVariantMap)
         p = pack_map(v.toMap(), p, wr);
     else {
-        if (user_packers.contains(t))
+        if (t == QMetaType::User)
+            t = (QMetaType::Type)v.userType();
+        packers_lock.lockForRead();
+        bool has_packer = user_packers.contains(t);
+        packers_lock.unlock();
+        if (has_packer)
             p = pack_user(v, p, wr);
         else
             qWarning() << "MsgPack::pack can't pack type:" << t;
@@ -269,21 +275,23 @@ quint8 *MsgPackPrivate::pack_map(const QVariantMap &map, quint8 *p, bool wr)
     return p;
 }
 
-
 bool MsgPackPrivate::register_packer(QMetaType::Type q_type, qint8 msgpack_type, MsgPack::pack_user_f packer)
 {
-    if (user_packers.contains(q_type)) {
-        qWarning() << "MsgPack::packer for qtype" << q_type << "already exist";
-        return false;
-    }
     if (packer == 0) {
         qWarning() << "MsgPack::packer for qtype" << q_type << "is invalid";
         return false;
     }
+    packers_lock.lockForWrite();
+    if (user_packers.contains(q_type)) {
+        qWarning() << "MsgPack::packer for qtype" << q_type << "already exist";
+        packers_lock.unlock();
+        return false;
+    }
     packer_t p;
     p.packer = packer;
     p.type = msgpack_type;
     user_packers.insert(q_type, p);
+    packers_lock.unlock();
     return true;
 }
 
@@ -292,7 +300,9 @@ quint8 *MsgPackPrivate::pack_user(const QVariant &v, quint8 *p, bool wr)
     QMetaType::Type t = (QMetaType::Type)v.type() == QMetaType::User ?
                 (QMetaType::Type)v.userType() : (QMetaType::Type)v.type();
     QByteArray data;
+    packers_lock.lockForRead();
     packer_t pt = user_packers[t];
+    packers_lock.unlock();
     quint32 len = pt.packer(v, data, wr);
     if (len == 1) {
         if (wr) *p = 0xd4;
@@ -331,4 +341,3 @@ quint8 *MsgPackPrivate::pack_user(const QVariant &v, quint8 *p, bool wr)
         memcpy(p, data.data(), len);
     return p += len;
 }
-

src/private/pack_p.h

@@ -5,6 +5,7 @@
 
 #include <QHash>
 #include <QMetaType>
+#include <QReadWriteLock>
 
 class QByteArray;
 class QString;
@@ -19,6 +20,7 @@ typedef struct {
 } packer_t;
 bool register_packer(QMetaType::Type q_type, qint8 msgpack_type, MsgPack::pack_user_f packer);
 extern QHash<QMetaType::Type, packer_t> user_packers;
+extern QReadWriteLock packers_lock;
 extern bool compatibilityMode;
 
 quint8 * pack(const QVariant &v, quint8 *p, bool wr);

src/private/unpack_p.cpp

@@ -21,6 +21,7 @@ MsgPackPrivate::type_parser_f MsgPackPrivate::unpackers[32] = {
 };
 
 QHash<qint8, MsgPack::unpack_user_f> MsgPackPrivate::user_unpackers;
+QReadWriteLock MsgPackPrivate::unpackers_lock;
 
 QVariant MsgPackPrivate::unpack(quint8 *p, quint8 *end)
 {
@@ -314,12 +315,14 @@ quint8 * MsgPackPrivate::unpack_map32(QVariant &v, quint8 *p)
 
 quint8 *MsgPackPrivate::unpack_ext(QVariant &v, quint8 *p, qint8 type, quint32 len)
 {
+    unpackers_lock.lockForRead();
     if (!user_unpackers.contains(type)) {
         qWarning() << "MsgPack::unpack() unpacker for type" << type << "doesn't exist";
         return p + len;
     }
     QByteArray data((char *)p, len);
     v = user_unpackers[type](data);
+    unpackers_lock.unlock();
     return p + len;
 }
 
@@ -382,14 +385,17 @@ quint8 * MsgPackPrivate::unpack_ext32(QVariant &v, quint8 *p)
 
 bool MsgPackPrivate::register_unpacker(qint8 msgpack_type, MsgPack::unpack_user_f unpacker)
 {
-    if (user_unpackers.contains(msgpack_type)) {
-        qWarning() << "MsgPack::unpacker for type" << msgpack_type << "already exists";
-        return false;
-    }
     if (unpacker == 0) {
         qWarning() << "MsgPack::unpacker for type" << msgpack_type << "is invalid";
         return false;
     }
+    unpackers_lock.lockForWrite();
+    if (user_unpackers.contains(msgpack_type)) {
+        qWarning() << "MsgPack::unpacker for type" << msgpack_type << "already exists";
+        unpackers_lock.unlock();
+        return false;
+    }
     user_unpackers.insert(msgpack_type, unpacker);
+    unpackers_lock.unlock();
     return true;
 }

src/private/unpack_p.h

@@ -5,6 +5,7 @@
 
 #include <QHash>
 #include <QVariant>
+#include <QReadWriteLock>
 
 namespace MsgPackPrivate
 {
@@ -19,6 +20,7 @@ extern type_parser_f unpackers[32];
 
 bool register_unpacker(qint8 msgpack_type, MsgPack::unpack_user_f unpacker);
 extern QHash<qint8, MsgPack::unpack_user_f> user_unpackers;
+extern QReadWriteLock unpackers_lock;
 
 // goes from p to end unpacking types with unpack_type function below
 QVariant unpack(quint8 *p, quint8 *end);

tests/mixed/mixed_test.cpp

@@ -90,9 +90,11 @@ void MixedTest::test_ext()
     QVariant custom;
     custom.setValue(ct);
 
-    MsgPack::registerPacker((QMetaType::Type)qMetaTypeId<CustomType>(),
+    bool packer_registered = MsgPack::registerPacker((QMetaType::Type)qMetaTypeId<CustomType>(),
                             3, pack_custom_type);
-    MsgPack::registerUnpacker(3, unpack_custom_type);
+    QVERIFY(packer_registered);
+    bool unpacker_registered = MsgPack::registerUnpacker(3, unpack_custom_type);
+    QVERIFY(unpacker_registered);
 
     QByteArray arr = MsgPack::pack(custom);
     QVERIFY(arr.size() == 2 + ct.size());