D27650: Don't use guarded pointers for AppletsLayout

Wed Feb 26 08:19:50 GMT 2020

alnikiforov added a comment.

  In D27650#617550 <https://phabricator.kde.org/D27650#617550>, @davidedmundson wrote:

  > > This function takes a plain pointer and wraps it into weak shared pointer.
  >
  > That's QWeakPointer.
  >
  > QPointer is something else that doesn't have an equivalent in stdlib.
  >  It takes a QObject,  then QObject has a special hook to unset watching QPointers to nullptr in QObject::~QObject()

  QPointer actually uses QWeakPointer to work:

  https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/kernel/qpointer.h#n59
  https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qsharedpointer_impl.h#n309
  https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qsharedpointer_impl.h#n450
  https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qsharedpointer_impl.h#n569

  And this used QWeakPointer leads to premature destruction of object m_layout points to.

  > I don't understand how this could fix anything.
  > 
  > If this is null before, then after this patch m_layout would just be dangling which is worse.

  I've debugged this issue a bit more. Here's what I found.

  Initial setup from gdb:

    (gdb) break ItemContainer::setLayout
    Breakpoint 1 at 0x7fffba5e2ea0: file /usr/src/debug/plasma-workspace-5.18.1/components/containmentlayoutmanager/itemcontainer.cpp, line 177.
    (gdb) run --replace
    Thread 1 "plasmashell" hit Breakpoint 1, ItemContainer::setLayout (this=this at entry=0xc01f80, layout=0x0) at /usr/src/debug/plasma-workspace-5.18.1/components/containmentlayoutmanager/itemcontainer.cpp:177
    177     {
    (gdb) c
    Continuing.

    Thread 1 "plasmashell" hit Breakpoint 1, ItemContainer::setLayout (this=0xc01f80, layout=0xbe0a60) at /usr/src/debug/plasma-workspace-5.18.1/components/containmentlayoutmanager/itemcontainer.cpp:177
    177     {
    (gdb) n
    178         if (m_layout == layout) {
    (gdb) n
    198         if (parentItem() != layout) {
    (gdb) n
    210         });
    (gdb) print m_layout
    $1 = {wp = {d = 0xc01bc0, value = 0xbe0a60}}
    (gdb) print m_layout.wp.d
    $2 = (QWeakPointer<QObject>::Data *) 0xc01bc0
    (gdb) print *m_layout.wp.d
    $3 = {weakref = {_q_value = {<std::__atomic_base<int>> = {static _S_alignment = 4, _M_i = 4}, <No data fields>}}, strongref = {_q_value = {<std::__atomic_base<int>> = {static _S_alignment = 4, _M_i = -1}, <No data fields>}}, 
      destroyer = 0x7ffff57ada40 <main_arena+96>}
    (gdb) watch ((QWeakPointer<QObject>::Data *) 0xc01bc0)->weakref._q_value._M_i
    Hardware watchpoint 2: ((QWeakPointer<QObject>::Data *) 0xc01bc0)->weakref._q_value._M_i
    (gdb) watch ((QWeakPointer<QObject>::Data *) 0xc01bc0)->strongref._q_value._M_i
    Hardware watchpoint 3: ((QWeakPointer<QObject>::Data *) 0xc01bc0)->strongref._q_value._M_i
    (gdb) break QObject::~QObject if (this == 0xbe0a60)
    Breakpoint 4 at 0x41c3e0 (75 locations)
    (gdb) c
    Continuing.

  I've added some widgets like described in test plan. Now I'm removing one of widgets:

    Thread 1 "plasmashell" hit Hardware watchpoint 2: ((QWeakPointer<QObject>::Data *) 0xc01bc0)->weakref._q_value._M_i

    Old value = 5
    New value = 4
    0x00007fffba5e4445 in QWeakPointer<QObject>::~QWeakPointer (this=0x10c2fc0, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/atomic_base.h:326
    326           operator--() noexcept
    (gdb) bt
    #0  0x00007fffba5e4445 in QWeakPointer<QObject>::~QWeakPointer (this=0x10c2fc0, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/atomic_base.h:326
    #1  QPointer<AppletsLayout>::~QPointer (this=0x10c2fc0, __in_chrg=<optimized out>) at /usr/include/qt5/QtCore/qpointer.h:53
    #2  ItemContainer::~ItemContainer (this=0x10c2f60, __in_chrg=<optimized out>) at /usr/src/debug/plasma-workspace-5.18.1/components/containmentlayoutmanager/itemcontainer.cpp:64
    #3  0x00007fffba5d30a5 in QQmlPrivate::QQmlElement<AppletContainer>::~QQmlElement (this=0x10c2f60, __in_chrg=<optimized out>) at /usr/include/qt5/QtQml/qqmlprivate.h:106
    #4  QQmlPrivate::QQmlElement<AppletContainer>::~QQmlElement (this=0x10c2f60, __in_chrg=<optimized out>) at /usr/include/qt5/QtQml/qqmlprivate.h:108
    #5  0x00007ffff5ce2380 in QObject::event (this=this at entry=0x10c2f60, e=e at entry=0x2b76cc0) at kernel/qobject.cpp:1252
    #6  0x00007ffff77c2ae3 in QQuickItem::event (this=0x10c2f60, ev=0x2b76cc0) at items/qquickitem.cpp:8105
    #7  0x00007ffff6711c42 in QApplicationPrivate::notify_helper (this=this at entry=0x4c14c0, receiver=receiver at entry=0x10c2f60, e=e at entry=0x2b76cc0) at kernel/qapplication.cpp:3700
    #8  0x00007ffff671b1c0 in QApplication::notify (this=0x7fffffffd730, receiver=0x10c2f60, e=0x2b76cc0) at kernel/qapplication.cpp:3446
    #9  0x00007ffff5cb7212 in QCoreApplication::notifyInternal2 (receiver=0x10c2f60, event=0x2b76cc0) at ../../src/corelib/kernel/qobject.h:142
    #10 0x00007ffff5cb9e08 in QCoreApplicationPrivate::sendPostedEvents (receiver=0x0, event_type=0, data=0x4b6330) at kernel/qcoreapplication.cpp:1825
    #11 0x00007ffff5d0d963 in postEventSourceDispatch (s=0x51a7c0) at kernel/qeventdispatcher_glib.cpp:276
    #12 0x00007ffff3b86c6d in g_main_dispatch (context=0x7fffe8005010) at ../glib/gmain.c:3179
    #13 g_main_context_dispatch (context=context at entry=0x7fffe8005010) at ../glib/gmain.c:3844
    #14 0x00007ffff3b86ef0 in g_main_context_iterate (context=context at entry=0x7fffe8005010, block=block at entry=1, dispatch=dispatch at entry=1, self=<optimized out>) at ../glib/gmain.c:3917
    #15 0x00007ffff3b86f7f in g_main_context_iteration (context=0x7fffe8005010, may_block=may_block at entry=1) at ../glib/gmain.c:3978
    #16 0x00007ffff5d0cfa1 in QEventDispatcherGlib::processEvents (this=0x51b110, flags=...) at kernel/qeventdispatcher_glib.cpp:422
    #17 0x00007ffff5cb5e9b in QEventLoop::exec (this=this at entry=0x7fffffffd5f0, flags=..., flags at entry=...) at ../../src/corelib/global/qflags.h:140
    #18 0x00007ffff5cbd942 in QCoreApplication::exec () at ../../src/corelib/global/qflags.h:120
    #19 0x00007ffff608ecbc in QGuiApplication::exec () at kernel/qguiapplication.cpp:1784
    #20 0x00007ffff6711bb5 in QApplication::exec () at kernel/qapplication.cpp:2856
    #21 0x000000000041f4f1 in main (argc=<optimized out>, argv=<optimized out>) at /usr/src/debug/plasma-workspace-5.18.1/shell/main.cpp:228
    (gdb) c
    Continuing.

    Thread 1 "plasmashell" hit Breakpoint 1, ItemContainer::setLayout (this=0x10c2f60, layout=0x0) at /usr/src/debug/plasma-workspace-5.18.1/components/containmentlayoutmanager/itemcontainer.cpp:177
    177     {
    (gdb) bt
    #0  ItemContainer::setLayout (this=0x10c2f60, layout=0x0) at /usr/src/debug/plasma-workspace-5.18.1/components/containmentlayoutmanager/itemcontainer.cpp:177
    #1  0x00007ffff5ce1c98 in QtPrivate::QSlotObjectBase::call (a=0x7fffffffcee0, r=0x10c2f60, this=0x2dcf4b0) at ../../src/corelib/kernel/qobjectdefs_impl.h:394
    #2  QMetaObject::activate (sender=0x10c2f60, signalOffset=<optimized out>, local_signal_index=<optimized out>, argv=<optimized out>) at kernel/qobject.cpp:3784
    #3  0x00007ffff77aedb2 in QQuickItem::parentChanged (this=this at entry=0x10c2f60, _t1=<optimized out>) at .moc/moc_qquickitem.cpp:1100
    #4  0x00007ffff77c3fdd in QQuickItem::setParentItem (this=this at entry=0x10c2f60, parentItem=parentItem at entry=0x0) at items/qquickitem.cpp:2808
    #5  0x00007ffff77c4c38 in QQuickItem::~QQuickItem (this=0x10c2f60, __in_chrg=<optimized out>) at items/qquickitem.cpp:2390
    #6  0x00007fffba5d30a5 in QQmlPrivate::QQmlElement<AppletContainer>::~QQmlElement (this=0x10c2f60, __in_chrg=<optimized out>) at /usr/include/qt5/QtQml/qqmlprivate.h:106
    #7  QQmlPrivate::QQmlElement<AppletContainer>::~QQmlElement (this=0x10c2f60, __in_chrg=<optimized out>) at /usr/include/qt5/QtQml/qqmlprivate.h:108
    #8  0x00007ffff5ce2380 in QObject::event (this=this at entry=0x10c2f60, e=e at entry=0x2b76cc0) at kernel/qobject.cpp:1252
    #9  0x00007ffff77c2ae3 in QQuickItem::event (this=0x10c2f60, ev=0x2b76cc0) at items/qquickitem.cpp:8105
    #10 0x00007ffff6711c42 in QApplicationPrivate::notify_helper (this=this at entry=0x4c14c0, receiver=receiver at entry=0x10c2f60, e=e at entry=0x2b76cc0) at kernel/qapplication.cpp:3700
    #11 0x00007ffff671b1c0 in QApplication::notify (this=0x7fffffffd730, receiver=0x10c2f60, e=0x2b76cc0) at kernel/qapplication.cpp:3446
    #12 0x00007ffff5cb7212 in QCoreApplication::notifyInternal2 (receiver=0x10c2f60, event=0x2b76cc0) at ../../src/corelib/kernel/qobject.h:142
    #13 0x00007ffff5cb9e08 in QCoreApplicationPrivate::sendPostedEvents (receiver=0x0, event_type=0, data=0x4b6330) at kernel/qcoreapplication.cpp:1825
    #14 0x00007ffff5d0d963 in postEventSourceDispatch (s=0x51a7c0) at kernel/qeventdispatcher_glib.cpp:276
    #15 0x00007ffff3b86c6d in g_main_dispatch (context=0x7fffe8005010) at ../glib/gmain.c:3179
    #16 g_main_context_dispatch (context=context at entry=0x7fffe8005010) at ../glib/gmain.c:3844
    #17 0x00007ffff3b86ef0 in g_main_context_iterate (context=context at entry=0x7fffe8005010, block=block at entry=1, dispatch=dispatch at entry=1, self=<optimized out>) at ../glib/gmain.c:3917
    #18 0x00007ffff3b86f7f in g_main_context_iteration (context=0x7fffe8005010, may_block=may_block at entry=1) at ../glib/gmain.c:3978
    #19 0x00007ffff5d0cfa1 in QEventDispatcherGlib::processEvents (this=0x51b110, flags=...) at kernel/qeventdispatcher_glib.cpp:422
    #20 0x00007ffff5cb5e9b in QEventLoop::exec (this=this at entry=0x7fffffffd5f0, flags=..., flags at entry=...) at ../../src/corelib/global/qflags.h:140
    #21 0x00007ffff5cbd942 in QCoreApplication::exec () at ../../src/corelib/global/qflags.h:120
    #22 0x00007ffff608ecbc in QGuiApplication::exec () at kernel/qguiapplication.cpp:1784
    #23 0x00007ffff6711bb5 in QApplication::exec () at kernel/qapplication.cpp:2856
    #24 0x000000000041f4f1 in main (argc=<optimized out>, argv=<optimized out>) at /usr/src/debug/plasma-workspace-5.18.1/shell/main.cpp:228
    (gdb) c
    Continuing.

    Thread 1 "plasmashell" hit Hardware watchpoint 2: ((QWeakPointer<QObject>::Data *) 0xc01bc0)->weakref._q_value._M_i

    Old value = 4
    New value = 3
    0x00007fffba5e2fd7 in QWeakPointer<QObject>::~QWeakPointer (this=<synthetic pointer>, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/atomic_base.h:326
    326           operator--() noexcept
    (gdb) bt
    #0  0x00007fffba5e2fd7 in QWeakPointer<QObject>::~QWeakPointer (this=<synthetic pointer>, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/atomic_base.h:326
    #1  QWeakPointer<QObject>::operator= (other=..., other=..., this=0x10c2fc0) at /usr/include/qt5/QtCore/qsharedpointer_impl.h:599
    #2  QWeakPointer<QObject>::assign<QObject> (ptr=0x0, this=0x10c2fc0) at /usr/include/qt5/QtCore/qsharedpointer_impl.h:684
    #3  QPointer<AppletsLayout>::operator= (p=0x0, this=0x10c2fc0) at /usr/include/qt5/QtCore/qpointer.h:83
    #4  ItemContainer::setLayout (this=0x10c2f60, layout=0x0) at /usr/src/debug/plasma-workspace-5.18.1/components/containmentlayoutmanager/itemcontainer.cpp:191
    #5  0x00007ffff5ce1c98 in QtPrivate::QSlotObjectBase::call (a=0x7fffffffcee0, r=0x10c2f60, this=0x2dcf4b0) at ../../src/corelib/kernel/qobjectdefs_impl.h:394
    #6  QMetaObject::activate (sender=0x10c2f60, signalOffset=<optimized out>, local_signal_index=<optimized out>, argv=<optimized out>) at kernel/qobject.cpp:3784
    #7  0x00007ffff77aedb2 in QQuickItem::parentChanged (this=this at entry=0x10c2f60, _t1=<optimized out>) at .moc/moc_qquickitem.cpp:1100
    #8  0x00007ffff77c3fdd in QQuickItem::setParentItem (this=this at entry=0x10c2f60, parentItem=parentItem at entry=0x0) at items/qquickitem.cpp:2808
    #9  0x00007ffff77c4c38 in QQuickItem::~QQuickItem (this=0x10c2f60, __in_chrg=<optimized out>) at items/qquickitem.cpp:2390
    #10 0x00007fffba5d30a5 in QQmlPrivate::QQmlElement<AppletContainer>::~QQmlElement (this=0x10c2f60, __in_chrg=<optimized out>) at /usr/include/qt5/QtQml/qqmlprivate.h:106
    #11 QQmlPrivate::QQmlElement<AppletContainer>::~QQmlElement (this=0x10c2f60, __in_chrg=<optimized out>) at /usr/include/qt5/QtQml/qqmlprivate.h:108
    #12 0x00007ffff5ce2380 in QObject::event (this=this at entry=0x10c2f60, e=e at entry=0x2b76cc0) at kernel/qobject.cpp:1252
    #13 0x00007ffff77c2ae3 in QQuickItem::event (this=0x10c2f60, ev=0x2b76cc0) at items/qquickitem.cpp:8105
    #14 0x00007ffff6711c42 in QApplicationPrivate::notify_helper (this=this at entry=0x4c14c0, receiver=receiver at entry=0x10c2f60, e=e at entry=0x2b76cc0) at kernel/qapplication.cpp:3700
    #15 0x00007ffff671b1c0 in QApplication::notify (this=0x7fffffffd730, receiver=0x10c2f60, e=0x2b76cc0) at kernel/qapplication.cpp:3446
    #16 0x00007ffff5cb7212 in QCoreApplication::notifyInternal2 (receiver=0x10c2f60, event=0x2b76cc0) at ../../src/corelib/kernel/qobject.h:142
    #17 0x00007ffff5cb9e08 in QCoreApplicationPrivate::sendPostedEvents (receiver=0x0, event_type=0, data=0x4b6330) at kernel/qcoreapplication.cpp:1825
    #18 0x00007ffff5d0d963 in postEventSourceDispatch (s=0x51a7c0) at kernel/qeventdispatcher_glib.cpp:276
    #19 0x00007ffff3b86c6d in g_main_dispatch (context=0x7fffe8005010) at ../glib/gmain.c:3179
    #20 g_main_context_dispatch (context=context at entry=0x7fffe8005010) at ../glib/gmain.c:3844
    #21 0x00007ffff3b86ef0 in g_main_context_iterate (context=context at entry=0x7fffe8005010, block=block at entry=1, dispatch=dispatch at entry=1, self=<optimized out>) at ../glib/gmain.c:3917
    #22 0x00007ffff3b86f7f in g_main_context_iteration (context=0x7fffe8005010, may_block=may_block at entry=1) at ../glib/gmain.c:3978
    #23 0x00007ffff5d0cfa1 in QEventDispatcherGlib::processEvents (this=0x51b110, flags=...) at kernel/qeventdispatcher_glib.cpp:422
    #24 0x00007ffff5cb5e9b in QEventLoop::exec (this=this at entry=0x7fffffffd5f0, flags=..., flags at entry=...) at ../../src/corelib/global/qflags.h:140
    #25 0x00007ffff5cbd942 in QCoreApplication::exec () at ../../src/corelib/global/qflags.h:120
    #26 0x00007ffff608ecbc in QGuiApplication::exec () at kernel/qguiapplication.cpp:1784
    #27 0x00007ffff6711bb5 in QApplication::exec () at kernel/qapplication.cpp:2856
    #28 0x000000000041f4f1 in main (argc=<optimized out>, argv=<optimized out>) at /usr/src/debug/plasma-workspace-5.18.1/shell/main.cpp:228
    (gdb) c

  As you may see, due to m_layout being a QPointer, which contains QWeakPointer, reference counter is decreased twice. If there is no QPointer, there is no double decrement and no issue. And m_layout doesn't become dangling pointer since object m_layout points to is created in qml and is not destroyed until desktop view is destroyed.

  Thus, while this fix is not perfect, it definitely fixes current issue.
  But looking into it closely after additional investigation, I think it might be fixed using another approach: signals have to be disconnected so that signal to already deconstructed object wouldn't be called from it's parent class. I'll upload updated version.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D27650

To: alnikiforov, ngraham, davidedmundson, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200226/e5620f45/attachment-0001.html>