From e7c6afcd4940952a915b640125089e31d45f78c1 Mon Sep 17 00:00:00 2001 From: Roman Date: Tue, 14 Apr 2015 22:50:20 +0300 Subject: [PATCH] Thread safety implemented for registerPacker and registerUnpacker --- src/private/pack_p.cpp | 23 ++++++++++++++++------- src/private/pack_p.h | 2 ++ src/private/unpack_p.cpp | 14 ++++++++++---- src/private/unpack_p.h | 2 ++ tests/mixed/mixed_test.cpp | 6 ++++-- 5 files changed, 34 insertions(+), 13 deletions(-) diff --git a/src/private/pack_p.cpp b/src/private/pack_p.cpp index 7e759d1..da38357 100644 --- a/src/private/pack_p.cpp +++ b/src/private/pack_p.cpp @@ -11,6 +11,7 @@ QHash 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; } - diff --git a/src/private/pack_p.h b/src/private/pack_p.h index b29f971..4ef57f4 100644 --- a/src/private/pack_p.h +++ b/src/private/pack_p.h @@ -5,6 +5,7 @@ #include #include +#include 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 user_packers; +extern QReadWriteLock packers_lock; extern bool compatibilityMode; quint8 * pack(const QVariant &v, quint8 *p, bool wr); diff --git a/src/private/unpack_p.cpp b/src/private/unpack_p.cpp index 511bd4a..4983d02 100644 --- a/src/private/unpack_p.cpp +++ b/src/private/unpack_p.cpp @@ -21,6 +21,7 @@ MsgPackPrivate::type_parser_f MsgPackPrivate::unpackers[32] = { }; QHash 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; } diff --git a/src/private/unpack_p.h b/src/private/unpack_p.h index ed8b323..ae85648 100644 --- a/src/private/unpack_p.h +++ b/src/private/unpack_p.h @@ -5,6 +5,7 @@ #include #include +#include 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 user_unpackers; +extern QReadWriteLock unpackers_lock; // goes from p to end unpacking types with unpack_type function below QVariant unpack(quint8 *p, quint8 *end); diff --git a/tests/mixed/mixed_test.cpp b/tests/mixed/mixed_test.cpp index 1976483..1debc23 100644 --- a/tests/mixed/mixed_test.cpp +++ b/tests/mixed/mixed_test.cpp @@ -90,9 +90,11 @@ void MixedTest::test_ext() QVariant custom; custom.setValue(ct); - MsgPack::registerPacker((QMetaType::Type)qMetaTypeId(), + bool packer_registered = MsgPack::registerPacker((QMetaType::Type)qMetaTypeId(), 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());