Merge lp:~aacid/gsettings-qt/disconnect_signal_handler into lp:gsettings-qt

Proposed by Albert Astals Cid
Status: Merged
Approved by: Lars Karlitski
Approved revision: 76
Merged at revision: 78
Proposed branch: lp:~aacid/gsettings-qt/disconnect_signal_handler
Merge into: lp:gsettings-qt
Diff against target: 27 lines (+3/-1)
1 file modified
src/qgsettings.cpp (+3/-1)
To merge this branch: bzr merge lp:~aacid/gsettings-qt/disconnect_signal_handler
Reviewer Review Type Date Requested Status
Lars Karlitski (community) Approve
Review via email:

Commit message

Disconnect signal handler on destruction

When running unity8 under valgrind sometimes I get

    ==31122== Invalid read of size 8
    ==31122== at 0x620DB42: QMetaObject::invokeMethod(QObject*, char const*, Qt::ConnectionType, QGenericReturnArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument) (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x1C7C63EE: invokeMethod (qobjectdefs.h:391)
    ==31122== by 0x1C7C63EE: QGSettingsPrivate::settingChanged(_GSettings*, char const*, void*) (qgsettings.cpp:40)
    ==31122== by 0x129DAA59: g_cclosure_marshal_VOID__STRINGv (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x129D81D3: _g_closure_invoke_va (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x129F29D5: g_signal_emit_valist (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x129F30BE: g_signal_emit (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x138478EB: g_settings_real_change_event (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x14540D8F: ffi_call_unix64 (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x145407F7: ffi_call (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x129D8CF4: g_cclosure_marshal_generic_va (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x129D81D3: _g_closure_invoke_va (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x129F24E7: g_signal_emit_valist (in /usr/lib/x86_64-linux-gnu/
    ==31122== Address 0x1dbaa450 is 0 bytes inside a block of size 24 free'd
    ==31122== at 0x4C2D28B: operator delete(void*) (vg_replace_malloc.c:575)
    ==31122== by 0x622CD2A: QObjectPrivate::deleteChildren() (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x623661F: QObject::~QObject() (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x1C5BB7AD: ~QQmlElement (qqmlprivate.h:98)
    ==31122== by 0x1C5BB7AD: QQmlPrivate::QQmlElement<GSettingsQml>::~QQmlElement() (qqmlprivate.h:98)
    ==31122== by 0x622CD2A: QObjectPrivate::deleteChildren() (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x623661F: QObject::~QObject() (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x53E886C: QQuickItem::~QQuickItem() (qquickitem.cpp:2223)
    ==31122== by 0x5402DED: ~QQuickRectangle (qquickrectangle_p.h:128)
    ==31122== by 0x5402DED: ~QQmlElement (qqmlprivate.h:98)
    ==31122== by 0x5402DED: QQmlPrivate::QQmlElement<QQuickRectangle>::~QQmlElement() (qqmlprivate.h:98)
    ==31122== by 0x622F66F: QObject::event(QEvent*) (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x53E66BA: QQuickItem::event(QEvent*) (qquickitem.cpp:7302)
    ==31122== by 0x61FDDE8: QCoreApplication::notify(QObject*, QEvent*) (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x61FDF1A: QCoreApplication::notifyInternal(QObject*, QEvent*) (in /usr/lib/x86_64-linux-gnu/
    ==31122== Block was alloc'd at
    ==31122== at 0x4C2C12F: operator new(unsigned long) (vg_replace_malloc.c:333)
    ==31122== by 0x1C5BD113: GSettingsQml::componentComplete() (gsettings-qml.cpp:141)
    ==31122== by 0x5E25421: ??? (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x5DB22E9: ??? (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x5DB2B9E: QQmlEnginePrivate::incubate(QQmlIncubator&, QQmlContextData*) (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x5DAE4EB: QQmlComponent::create(QQmlIncubator&, QQmlContext*, QQmlContext*) (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x5498859: QQuickLoaderPrivate::_q_sourceLoaded() (qquickloader.cpp:713)
    ==31122== by 0x54989F7: QQuickLoaderPrivate::load() (qquickloader.cpp:596)
    ==31122== by 0x5E25421: ??? (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x5DADEAD: QQmlComponentPrivate::complete(QQmlEnginePrivate*, QQmlComponentPrivate::ConstructionState*) (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x5DADF76: QQmlComponentPrivate::completeCreate() (in /usr/lib/x86_64-linux-gnu/
    ==31122== by 0x5DADDDF: QQmlComponent::create(QQmlContext*) (in /usr/lib/x86_64-linux-gnu/

Which as far as i understand means that the QGSettingsPrivate::settingChanged is being called even if GSettingsQml has already been destroyed.

By disconnecting the signal handler I have not been able to reproduce the valgrind warning anymore

To post a comment you must log in.
Revision history for this message
Lars Karlitski (larsu) wrote :

Weird. I assumed this cannot happen since we're not passing a ref to the settings object anywhere else and it gets destroyed in the destructor.

Thanks for the catch!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/qgsettings.cpp'
2--- src/qgsettings.cpp 2015-06-02 10:10:03 +0000
3+++ src/qgsettings.cpp 2015-11-30 11:11:41 +0000
4@@ -28,6 +28,7 @@
5 QByteArray path;
6 GSettings *settings;
7 GSettingsSchema *schema;
8+ gulong signal_handler_id;
10 static void settingChanged(GSettings *settings, const gchar *key, gpointer user_data);
11 };
12@@ -53,13 +54,14 @@
13 priv->settings = g_settings_new_with_path(priv->schema_id.constData(), priv->path.constData());
15 g_object_get (priv->settings, "settings-schema", &priv->schema, NULL);
16- g_signal_connect(priv->settings, "changed", G_CALLBACK(QGSettingsPrivate::settingChanged), this);
17+ priv->signal_handler_id = g_signal_connect(priv->settings, "changed", G_CALLBACK(QGSettingsPrivate::settingChanged), this);
18 }
20 QGSettings::~QGSettings()
21 {
22 if (priv->schema) {
23 g_settings_sync ();
24+ g_signal_handler_disconnect(priv->settings, priv->signal_handler_id);
25 g_object_unref (priv->settings);
26 g_settings_schema_unref (priv->schema);
27 }


People subscribed via source and target branches