event->redactedBecause() can return a pointer to a deleted event

[nvrWhere]

Describe the bug

We have the following code in neochat for visualising a redacted event in the timeline.

auto reason = m_event->redactedBecause()->reason();
return (reason.isEmpty()) ? i18n("<i>[This message was deleted]</i>")
     : i18n("<i>[This message was deleted: %1]</i>", m_event->redactedBecause()->reason());

I've noticed a crash when entering a room and loading the timeline which is related to this code, suggesting that a pointer is being returned to an event that has already been deleted.

To Reproduce
Steps to reproduce the behaviour, and the description of the actual result:

1. Enter room with redacted event in timeline
2. Crash segfault

Expected behavior
No Crash

Is it environment-specific?
I assume not

Additional context
Backtrace:

#0 std::__lower_bound<QJsonPrivate::ObjectIterator<const QtCbor::Element, QListQtCbor::Element::const_iterator>, QLatin1String, __gnu_cxx::__ops::_Iter_comp_val<indexOf(const QExplicitlySharedDataPointer&, QLatin1String, bool*)::<lambda(const QJsonPrivate::ObjectIterator<const QtCbor::Element, QListQtCbor::Element::const_iterator>::value_type&, const QLatin1String&)> > >
(__first=..., __last=..., __val=..., __comp=...) at /usr/include/c++/14/bits/stl_algobase.h:1501
#1 std::lower_bound<QJsonPrivate::ObjectIterator<const QtCbor::Element, QListQtCbor::Element::const_iterator>, QLatin1String, indexOf(const QExplicitlySharedDataPointer&, QLatin1String, bool*)::<lambda(const QJsonPrivate::ObjectIterator<const QtCbor::Element, QListQtCbor::Element::const_iterator>::value_type&, const QLatin1String&)> > (__first=..., __last=..., __val=..., __comp=...)
at /usr/include/c++/14/bits/stl_algo.h:1973
#2 indexOf (o=..., key=..., keyExists=keyExists@entry=0x7fffffffae5f)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/serialization/qjsonobject.cpp:264
#3 0x00007ffff3e425fb in QJsonObject::valueImpl (this=0x1c7e8cf8, key=...)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/serialization/qjsonobject.cpp:314
#4 QJsonObject::value (this=0x1c7e8cf8, key=...)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/serialization/qjsonobject.cpp:301
#5 0x00007ffff67980c9 in QJsonObject::operator[] (this=, key=...)
at /usr/include/qt6/QtCore/qjsonobject.h:61
#6 Quotient::Event::contentJson (this=)
at /home/jgraham/kde/src/libquotient/Quotient/events/event.cpp:68
#7 0x00000000005c309e in Quotient::Event::contentPart<QString, QString const&> (this=, key=...)
at /home/jgraham/kde/usr/include/Quotient/events/event.h:363
#8 Quotient::RedactionEvent::reason (this=)
at /home/jgraham/kde/usr/include/Quotient/events/redactionevent.h:19
#9 0x00000000005bedf3 in MessageContentModel::data (this=0x1df8e0d0, index=..., role=)
at /home/jgraham/kde/src/neochat/src/models/messagecontentmodel.cpp:193
#10 0x00007ffff71156a8 in QModelIndex::data (this=0x7fffffffb080, arole=0)
at /usr/include/qt6/QtCore/qabstractitemmodel.h:493
#11 QQmlDMAbstractItemModelData::value (this=this@entry=0x1e28a560, role=0)
at /usr/src/debug/qt6-qtdeclarative-6.7.1-2.fc40.x86_64/src/qmlmodels/qqmldmabstractitemmodeldata.cpp:229
#12 0x00007ffff7115cfe in QQmlDMAbstractItemModelData::metaCall
(this=0x1e28a560, call=, id=, arguments=0x7fffffffb1d0)
at /usr/src/debug/qt6-qtdeclarative-6.7.1-2.fc40.x86_64/src/qmlmodels/qqmldmabstractitemmodeldata.cpp:30
#13 0x00007ffff3da7b11 in QMetaProperty::read (this=this@entry=0x7fffffffb320, object=0x1e28a560)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qmetaobject.cpp:3734
#14 0x00007ffff6de79ee in QQmlPropertyToPropertyBinding::update (this=0x1edae540, flags=...)
at /usr/src/debug/qt6-qtdeclarative-6.7.1-2.fc40.x86_64/src/qml/qml/qqmlpropertytopropertybinding.cpp:112
#15 0x00007ffff7105ca6 in QQDMIncubationTask::initializeRequiredProperties
(this=, modelItemToIncubate=, object=)
at /usr/src/debug/qt6-qtdeclarative-6.7.1-2.fc40.x86_64/src/qmlmodels/qqmldelegatemodel.cpp:984
#16 0x00007ffff7105e07 in QQmlDelegateModelPrivate::setInitialState
(this=0x1eb19a90, incubationTask=0x1e2749d0, o=0x1e0166e0)
at /usr/src/debug/qt6-qtdeclarative-6.7.1-2.fc40.x86_64/src/qmlmodels/qqmldelegatemodel.cpp:1160
#17 0x00007ffff6d63a63 in QQmlIncubatorPrivate::incubate (this=this@entry=0x1c7d1470, i=...)
at /usr/src/debug/qt6-qtdeclarative-6.7.1-2.fc40.x86_64/src/qml/qml/qqmlincubator.cpp:321
#18 0x00007ffff6d63daf in QQmlEnginePrivate::incubate (this=0x11448c0, i=, forContext=)
at /usr/src/debug/qt6-qtdeclarative-6.7.1-2.fc40.x86_64/src/qml/qml/qqmlincubator.cpp:53
#19 0x00007ffff71026c9 in QQmlDelegateModelPrivate::object
(this=0x1eb19a90, group=QQmlListCompositor::Default, index=1, incubationMode=)
at /usr/src/debug/qt6-qtdeclarative-6.7.1-2.fc40.x86_64/src/qmlmodels/qqmldelegatemodel.cpp:1284
--Type for more, q to quit, c to continue without paging--
#20 0x00007ffff7767240 in QQuickRepeaterPrivate::requestItems (this=0x1ed5cad0)
at /usr/src/debug/qt6-qtdeclarative-6.7.1-2.fc40.x86_64/src/quick/items/qquickrepeater.cpp:367
#21 0x00007ffff776a02c in QQuickRepeater::modelUpdated (this=0x1e8edf70, changeSet=..., reset=)
at /usr/src/debug/qt6-qtdeclarative-6.7.1-2.fc40.x86_64/src/quick/items/qquickrepeater.cpp:435
#22 0x00007ffff776a704 in QQuickRepeater::qt_metacall
(this=0x1e8edf70, _c=QMetaObject::InvokeMetaMethod, _id=7, _a=0x7fffffffba90)
at /usr/src/debug/qt6-qtdeclarative-6.7.1-2.fc40.x86_64/redhat-linux-build/src/quick/Quick_autogen/include/moc_qquickrepeater_p.cpp:297
#23 0x00007ffff3dfaa3a in doActivate (sender=0x1c285bc0, signal_index=4, argv=0x7fffffffba90)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qobject.cpp:4112
#24 0x00007ffff3df0b47 in QMetaObject::activate
(sender=, m=m@entry=0x7ffff714d7a0 QQmlInstanceModel::staticMetaObject, local_signal_index=local_signal_index@entry=1, argv=argv@entry=0x7fffffffba90)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qobject.cpp:4146
#25 0x00007ffff70b9bd7 in QQmlInstanceModel::modelUpdated
(this=, _t1=, _t2=)
at /usr/src/debug/qt6-qtdeclarative-6.7.1-2.fc40.x86_64/redhat-linux-build/src/qmlmodels/QmlModels_autogen/include/moc_qqmlobjectmodel_p.cpp:279
#26 0x00007ffff70f6c6d in non-virtual thunk to QQmlDelegateModelPrivate::emitModelUpdated(QQmlChangeSet const&, bool)
() at /usr/src/debug/qt6-qtdeclarative-6.7.1-2.fc40.x86_64/src/qmlmodels/qqmldelegatemodel_p_p.h:303
#27 0x00007ffff70fbb9b in QQmlDelegateModelGroupPrivate::emitModelUpdated (this=0x1c702830, reset=reset@entry=true)
at /usr/src/debug/qt6-qtdeclarative-6.7.1-2.fc40.x86_64/src/qmlmodels/qqmldelegatemodel.cpp:2871
#28 0x00007ffff70fe078 in QQmlDelegateModelPrivate::emitChanges (this=this@entry=0x1eb19a90)
at /usr/src/debug/qt6-qtdeclarative-6.7.1-2.fc40.x86_64/src/qmlmodels/qqmldelegatemodel.cpp:1893
#29 0x00007ffff710b667 in QQmlDelegateModel::handleModelReset (this=)
at /usr/src/debug/qt6-qtdeclarative-6.7.1-2.fc40.x86_64/src/qmlmodels/qqmldelegatemodel.cpp:1970
#30 0x00007ffff3dfa752 in QtPrivate::QSlotObjectBase::call (this=0x1fad6820, r=, a=0x7fffffffcd30)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qobjectdefs_impl.h:469
#31 doActivate (sender=0x1df8e0d0, signal_index=21, argv=0x7fffffffcd30)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qobject.cpp:4086
#32 0x00007ffff3df0b47 in QMetaObject::activate
(sender=, m=m@entry=0x7ffff4285fe0, local_signal_index=local_signal_index@entry=18, argv=argv@entry=0x7fffffffcd30) at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qobject.cpp:4146
#33 0x00007ffff4000ba0 in QAbstractItemModel::modelReset (this=, _t1=...)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/redhat-linux-build/src/corelib/Core_autogen/include/moc_qabstractitemmodel.cpp:1112
#34 0x00000000005bd7fc in operator() (_closure=0x1edd24f0)
at /home/jgraham/kde/src/neochat/src/models/messagecontentmodel.cpp:449
#35 QtPrivate::FunctorCall<QtPrivate::IndexesList<>, QtPrivate::List<>, void, MessageContentModel::linkPreviewComponent(const QUrl&)::<lambda()> >::call (f=..., arg=) at /usr/include/qt6/QtCore/qobjectdefs_impl.h:137
#36 QtPrivate::FunctorCallable<MessageContentModel::linkPreviewComponent(const QUrl&)::<lambda()> >::call<QtPrivate::List<>, void> (f=..., arg=) at /usr/include/qt6/QtCore/qobjectdefs_impl.h:345
#37 QtPrivate::QCallableObject<MessageContentModel::linkPreviewComponent(const QUrl&)::<lambda()>, QtPrivate::List<>, void>::impl(int, QtPrivate::QSlotObjectBase *, QObject *, void **, bool *)
(which=, this=0x1edd24e0, r=, a=, ret=)
at /usr/include/qt6/QtCore/qobjectdefs_impl.h:555
#38 0x00007ffff3dfa752 in QtPrivate::QSlotObjectBase::call (this=0x1edd24e0, r=, a=0x7fffffffce78)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qobjectdefs_impl.h:469
#39 doActivate (sender=0x1f9ac7e0, signal_index=3, argv=0x7fffffffce78)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qobject.cpp:4086
--Type for more, q to quit, c to continue without paging--
#40 0x00007ffff3df0b47 in QMetaObject::activate
(sender=, m=m@entry=0x86db40 LinkPreviewer::staticMetaObject, local_signal_index=local_signal_index@entry=0, argv=argv@entry=0x0) at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qobject.cpp:4146
#41 0x00000000006eb62f in LinkPreviewer::loadedChanged (this=)
at /home/jgraham/kde/build/neochat/src/neochat_autogen/include/moc_linkpreviewer.cpp:256
#42 operator() (__closure=0x1c82a090) at /home/jgraham/kde/src/neochat/src/linkpreviewer.cpp:82
#43 0x00007ffff3dfa752 in QtPrivate::QSlotObjectBase::call (this=0x1c82a080, r=, a=0x7fffffffd080)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qobjectdefs_impl.h:469
#44 doActivate (sender=0x1eb85070, signal_index=10, argv=0x7fffffffd080)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qobject.cpp:4086
#45 0x00007ffff3df0b47 in QMetaObject::activate
(sender=, m=m@entry=0x7ffff687adc0, local_signal_index=local_signal_index@entry=7, argv=argv@entry=0x7fffffffd080) at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qobject.cpp:4146
#46 0x00007ffff66e9752 in Quotient::BaseJob::success (this=, _t1=)
at /home/jgraham/kde/build/libquotient/QuotientQt6_autogen/T4CFEN5LXH/moc_basejob.cpp:564
#47 0x00007ffff67aff95 in Quotient::BaseJob::finishJob (this=0x1eb85070)
at /home/jgraham/kde/src/libquotient/Quotient/jobs/basejob.cpp:641
#48 0x00007ffff3dfa752 in QtPrivate::QSlotObjectBase::call (this=0x1e85ec10, r=, a=0x7fffffffd1d8)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qobjectdefs_impl.h:469
#49 doActivate (sender=0x7fff64d9f8d0, signal_index=12, argv=0x7fffffffd1d8)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qobject.cpp:4086
#50 0x00007ffff3df0b47 in QMetaObject::activate
(sender=sender@entry=0x7fff64d9f8d0, m=m@entry=0x7ffff45f1660, local_signal_index=local_signal_index@entry=3, argv=argv@entry=0x0) at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qobject.cpp:4146
#51 0x00007ffff4492c77 in QNetworkReply::finished (this=this@entry=0x7fff64d9f8d0)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/redhat-linux-build/src/network/Network_autogen/include/moc_qnetworkreply.cpp:435
#52 0x00007ffff453af09 in QNetworkReplyHttpImplPrivate::finished (this=0x1df901c0)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/network/access/qnetworkreplyhttpimpl.cpp:2147
#53 0x00007ffff3debdeb in QObject::event (this=0x7fff64d9f8d0, e=0x7fff2408fef0)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qobject.cpp:1452
#54 0x00007ffff538b168 in QApplicationPrivate::notify_helper
(this=, receiver=0x7fff64d9f8d0, e=0x7fff2408fef0)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/widgets/kernel/qapplication.cpp:3287
#55 0x00007ffff3d95b18 in QCoreApplication::notifyInternal2 (receiver=0x7fff64d9f8d0, event=0x7fff2408fef0)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qcoreapplication.cpp:1134
#56 0x00007ffff3d95d7d in QCoreApplication::sendEvent (receiver=, event=)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qcoreapplication.cpp:1575
#57 0x00007ffff3d998c1 in QCoreApplicationPrivate::sendPostedEvents (receiver=0x0, event_type=0, data=0xa910a0)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qcoreapplication.cpp:1932
#58 0x00007ffff3d99b6d in QCoreApplication::sendPostedEvents (receiver=, event_type=)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qcoreapplication.cpp:1789
#59 0x00007ffff407d39f in postEventSourceDispatch (s=0xb0afa0)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qeventdispatcher_glib.cpp:244
#60 0x00007ffff130ee8c in g_main_dispatch (context=0x7fffd8000f00) at ../glib/gmain.c:3344
#61 g_main_context_dispatch_unlocked (context=0x7fffd8000f00) at ../glib/gmain.c:4152
#62 0x00007ffff1370c98 in g_main_context_iterate_unlocked.isra.0
(context=context@entry=0x7fffd8000f00, block=block@entry=1, dispatch=dispatch@entry=1, self=)
at ../glib/gmain.c:4217
#63 0x00007ffff1310383 in g_main_context_iteration (context=0x7fffd8000f00, may_block=1) at ../glib/gmain.c:4282
--Type for more, q to quit, c to continue without paging--
#64 0x00007ffff407cb53 in QEventDispatcherGlib::processEvents (this=0xa7ae40, flags=...)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/kernel/qeventdispatcher_glib.cpp:394
#65 0x00007ffff3da2713 in QEventLoop::exec (this=this@entry=0x7fffffffd750, flags=..., flags@entry=...)
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/global/qflags.h:34
#66 0x00007ffff3d9e69c in QCoreApplication::exec ()
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/corelib/global/qflags.h:74
#67 0x00007ffff47d53dd in QGuiApplication::exec ()
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/gui/kernel/qguiapplication.cpp:1926
#68 0x00007ffff538b0d9 in QApplication::exec ()
at /usr/src/debug/qt6-qtbase-6.7.1-2.fc40.x86_64/src/widgets/kernel/qapplication.cpp:2555
#69 0x000000000043a4d4 in main (argc=, argv=)
at /home/jgraham/kde/src/neochat/src/main.cpp:310

[KitsuneRal]

Interesting. _redactedBecause is a unique pointer, and nobody else does anything lifecycle-related to it. Basically, it is never deleted apart from the event that contains it. Are you sure that m_event is valid?

[TobiasFella]

I have a suspicion that we indeed have invalid events on the neochat side, see e.g. https://bugs.kde.org/show_bug.cgi?id=488066

[KitsuneRal]

If that's any help, I remember fighting a very nasty race between updating a room pointer in the event model and accessing the model items, in Quaternion. The problem was that the moment you change the room pointer, all those bindings in QML get triggered and some of them may very well concern events from the old room because endModelReset() hasn't been called yet, so the view assumes the old items are still valid (beginModelReset() doesn't invalidate anything, as one would expect). I ended up having not one but two model resets for that reason - first to neutral (nullptr) state, and then to the new room - to make sure QML doesn't have any leftovers from the old room.

[nvrWhere]

I created https://invent.kde.org/network/neochat/-/merge_requests/1790 copying Quaternion, lets see if it helps

[TobiasFella]

I think what's happening here is not a bug in libQuotient but rather:

• NeoChat stores pointer to event
• Event is replaced
• libQuotient deletes the event and notifies us that the event changes
• We're accessing the event
• Boom