QtDBus: Make access to spy hook list thread safe

Introduce a mutex that protects access to the spy hooks list.

Also don't pass around raw pointers to the spy hook list,
those can change when new hooks are added and storage is
reallocated. Change the type of the hooks list to QList
to take advantage of copy-on-write in common case.

Check whether the global static is destroyed in qDBusAddSpyHook().

Change-Id: I440a88ce088d6fb5817660c8e1e02901281b842f
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Ievgenii Meshcheriakov 2023-08-08 14:18:49 +02:00
parent 3c38efbfe4
commit 50aba545ff
2 changed files with 34 additions and 17 deletions

View File

@ -101,7 +101,12 @@ void qdbusDefaultThreadDebug(int action, int condition, QDBusConnectionPrivate *
qdbusThreadDebugFunc qdbusThreadDebug = nullptr;
#endif
typedef QVarLengthArray<QDBusSpyCallEvent::Hook, 4> QDBusSpyHookList;
struct QDBusSpyHookList
{
QBasicMutex lock;
QList<QDBusSpyCallEvent::Hook> list;
};
Q_GLOBAL_STATIC(QDBusSpyHookList, qDBusSpyHookList)
extern "C" {
@ -455,7 +460,12 @@ static QDBusConnectionPrivate::ArgMatchRules matchArgsForService(const QString &
extern Q_DBUS_EXPORT void qDBusAddSpyHook(QDBusSpyCallEvent::Hook);
void qDBusAddSpyHook(QDBusSpyCallEvent::Hook hook)
{
qDBusSpyHookList()->append(hook);
auto *hooks = qDBusSpyHookList();
if (!hooks)
return;
const auto locker = qt_scoped_lock(hooks->lock);
hooks->list.append(hook);
}
QDBusSpyCallEvent::~QDBusSpyCallEvent()
@ -470,14 +480,25 @@ QDBusSpyCallEvent::~QDBusSpyCallEvent()
void QDBusSpyCallEvent::placeMetaCall(QObject *)
{
invokeSpyHooks(msg, hooks, hookCount);
invokeSpyHooks(msg);
}
inline void QDBusSpyCallEvent::invokeSpyHooks(const QDBusMessage &msg, const Hook *hooks, int hookCount)
inline void QDBusSpyCallEvent::invokeSpyHooks(const QDBusMessage &msg)
{
// call the spy hook list
for (int i = 0; i < hookCount; ++i)
hooks[i](msg);
if (!qDBusSpyHookList.exists())
return;
// Create a copy of the hook list here, so that the hooks can be called
// without holding the lock.
QList<Hook> hookListCopy;
{
auto *hooks = qDBusSpyHookList();
const auto locker = qt_scoped_lock(hooks->lock);
hookListCopy = hooks->list;
}
for (auto hook : std::as_const(hookListCopy))
hook(msg);
}
extern "C" {
@ -526,15 +547,14 @@ bool QDBusConnectionPrivate::handleMessage(const QDBusMessage &amsg)
// a) if it's a local message, we're in the caller's thread, so invoke the filter directly
// b) if it's an external message, post to the main thread
if (Q_UNLIKELY(qDBusSpyHookList.exists()) && qApp) {
const QDBusSpyHookList &list = *qDBusSpyHookList;
if (isLocal) {
Q_ASSERT(QThread::currentThread() != thread());
qDBusDebug() << this << "invoking message spies directly";
QDBusSpyCallEvent::invokeSpyHooks(amsg, list.constData(), list.size());
QDBusSpyCallEvent::invokeSpyHooks(amsg);
} else {
qDBusDebug() << this << "invoking message spies via event";
QCoreApplication::postEvent(qApp, new QDBusSpyCallEvent(this, QDBusConnection(this),
amsg, list.constData(), list.size()));
QCoreApplication::postEvent(
qApp, new QDBusSpyCallEvent(this, QDBusConnection(this), amsg));
// we'll be called back, so return
return true;

View File

@ -117,18 +117,15 @@ class QDBusSpyCallEvent : public QAbstractMetaCallEvent
{
public:
typedef void (*Hook)(const QDBusMessage&);
QDBusSpyCallEvent(QDBusConnectionPrivate *cp, const QDBusConnection &c, const QDBusMessage &msg,
const Hook *hooks, int count)
: QAbstractMetaCallEvent(cp, 0), conn(c), msg(msg), hooks(hooks), hookCount(count)
QDBusSpyCallEvent(QDBusConnectionPrivate *cp, const QDBusConnection &c, const QDBusMessage &msg)
: QAbstractMetaCallEvent(cp, 0), conn(c), msg(msg)
{}
~QDBusSpyCallEvent() override;
void placeMetaCall(QObject *) override;
static inline void invokeSpyHooks(const QDBusMessage &msg, const Hook *hooks, int hookCount);
static inline void invokeSpyHooks(const QDBusMessage &msg);
QDBusConnection conn; // keeps the refcount in QDBusConnectionPrivate up
QDBusMessage msg;
const Hook *hooks;
int hookCount;
};
QT_END_NAMESPACE