Crash in EditableModelView when using keyboard shortcuts on a new row

[welwood08]

Checklist

• I'm reporting a problem with Chatterino
• I've verified that I'm running the most recent nightly build or stable release
• I've looked for my problem on the wiki
• I've searched the issues and pull requests for similar looking reports

Describe your issue

I was adding a Live Notification and encountered a crash when attempting to accomplish the task more quickly than usual. After experimenting, I have confirmed the bug also occurs in the Nicknames and Moderation buttons tables and therefore I conclude the issue is common to other uses of EditableModelView.

Steps to reproduce:

• Open the Settings window.
• In Nicknames, click Add. If the table was empty, click Add again.
• Double click on the newest row to start editing.
• Use the keyboard shortcut to Remove (Alt+R) or Move up (Alt+M).
• Observe assertion failure and subsequent crash:

/usr/include/c++/14.1.1/bits/stl_vector.h:1130: constexpr std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::operator[](size_type) [with _Tp = chatterino::Nickname; _Alloc = std::allocator<chatterino::Nickname>; reference = chatterino::Nickname&; size_type = long unsigned int]: Assertion '__n < this->size()' failed.

Reproducing on a different table produces a similar assertion referring to the contents of that table (I don't know the appropriate C++ terminology), ie the chatterino::Nickname parts become chatterino::LiveNotification etc.

This crash does not occur when clicking those buttons, only when using the keyboard shortcuts. This crash does not occur for the first row added to an empty table.

Screenshots

No response

OS and Chatterino Version

Chatterino 2.5.1 (commit 6e104bd) built with Qt 6.7.2 Running on Manjaro Linux, kernel: 6.6.44-1-MANJARO, DE: KDE Plasma 6.0.5

[Mm2PL]

There is no accelerators on 6e104bd or current master. Could you please share more information about your DE and its configuration regarding accelerators? However I can reproduce with the changes available on the test_buggy_accelerators branch. Your DE seems to be doing this I guess?

[welwood08]

My apologies, I was unaware of the possibility for that to be a feature of my desktop environment. If that's the case, I'm using KDE Plasma 6.0.5 and have not configured anything specific to Chatterino, nor have I deliberately configured Alt+R or Alt+M to do anything globally (those letters are just underlined on the buttons the same as other accelerator keys).

[Nerixyz]

A stack trace would be really helpful if you have one.

[welwood08]

I'm unsure how to generate one on my system, the wiki only seems to describe the procedure on Windows.

[Mm2PL]

I don't really have the time to fully diagnose and fix this but here is the stack trace. This is a similar but not exactly the same crash, Arch, built on the test branch. This was removing a Nickname while editing one.
Repo root path was long so I replaced it with :/.

Stacktrace

chatterino: :/src/common/SignalVector.hpp:119: void chatterino::SignalVector<T>::removeAt(int, void*) [with T = chatterino::Nickname]: Warunek zapewnienia `index >= 0 && index < int(this->items_.size())' nie został spełniony.
// index was 1, and this->items_ contained a single Nickname that wasn't the one I edited.

(gdb) bt
#0  0x00007ffff5ea53f4 in ??? () at /usr/lib/libc.so.6
#1  0x00007ffff5e4c120 in raise () at /usr/lib/libc.so.6
#2  0x00007ffff5e334c3 in abort () at /usr/lib/libc.so.6
#3  0x00007ffff5e333df in ??? () at /usr/lib/libc.so.6
#4  0x00007ffff5e44177 in __assert_fail () at /usr/lib/libc.so.6
#5  0x00005555557ead36 in chatterino::SignalVector<chatterino::Nickname>::removeAt (this=0x7fffffffd1b0, index=1, caller=0x7fffe4002740) at :/src/common/SignalVector.hpp:119
#6  0x00005555557e9c09 in chatterino::SignalVectorModel<chatterino::Nickname>::setData (this=0x7fffe4002740, index=..., value=..., role=2) at :/src/common/SignalVectorModel.hpp:174
#7  0x00007ffff7bf76d9 in QStyledItemDelegate::setModelData(QWidget*, QAbstractItemModel*, QModelIndex const&) const () at /usr/lib/libQt5Widgets.so.5
#8  0x00007ffff7bd57f7 in QAbstractItemView::commitData(QWidget*) () at /usr/lib/libQt5Widgets.so.5
#9  0x00007ffff7bda18f in QAbstractItemView::currentChanged(QModelIndex const&, QModelIndex const&) () at /usr/lib/libQt5Widgets.so.5
#10 0x00007ffff6cdfa62 in ??? () at /usr/lib/libQt5Core.so.5
#11 0x00007ffff6c6d791 in QItemSelectionModel::currentChanged(QModelIndex const&, QModelIndex const&) () at /usr/lib/libQt5Core.so.5
#12 0x00007ffff6c6ff36 in QItemSelectionModel::setCurrentIndex(QModelIndex const&, QFlags<QItemSelectionModel::SelectionFlag>) () at /usr/lib/libQt5Core.so.5
#13 0x00007ffff7bcb6bf in QAbstractItemView::setCurrentIndex(QModelIndex const&) () at /usr/lib/libQt5Widgets.so.5
#14 0x00007ffff7bd7ae7 in QAbstractItemView::rowsAboutToBeRemoved(QModelIndex const&, int, int) () at /usr/lib/libQt5Widgets.so.5
#15 0x00007ffff6cdfa62 in ??? () at /usr/lib/libQt5Core.so.5
#16 0x00007ffff6c639bb in QAbstractItemModel::rowsAboutToBeRemoved(QModelIndex const&, int, int, QAbstractItemModel::QPrivateSignal) () at /usr/lib/libQt5Core.so.5
#17 0x00007ffff6c63a4b in QAbstractItemModel::beginRemoveRows(QModelIndex const&, int, int) () at /usr/lib/libQt5Core.so.5
#18 0x0000555555e642fd in chatterino::SignalVectorModel<chatterino::Nickname>::initialize(chatterino::SignalVector<chatterino::Nickname>*)::{lambda(auto:1)#1}::operator()<chatterino::SignalVectorItemEvent<chatterino::Nickname> >(chatterino::SignalVectorItemEvent<chatterino::Nickname>) const
    (__closure=0x55555a8494f0, args=...) at :/src/common/SignalVectorModel.hpp:83
#19 0x0000555555e66a14 in std::__invoke_impl<void, chatterino::SignalVectorModel<chatterino::Nickname>::initialize(chatterino::SignalVector<chatterino::Nickname>*)::{lambda(auto:1)#1}&, chatterino::SignalVectorItemEvent<chatterino::Nickname> >(std::__invoke_other, chatterino::SignalVectorModel<chatterino::Nickname>::initialize(chatterino::SignalVector<chatterino::Nickname>*)::{lambda(auto:1)#1}&, chatterino::SignalVectorItemEvent<chatterino::Nickname>&&) (__f=...) at /usr/include/c++/14.2.1/bits/invoke.h:61
#20 0x0000555555e661c1 in std::__invoke_r<void, chatterino::SignalVectorModel<chatterino::Nickname>::initialize(chatterino::SignalVector<chatterino::Nickname>*)::{lambda(auto:1)#1}&, chatterino::SignalVectorItemEvent<chatterino::Nickname> >(chatterino::SignalVectorModel<chatterino::Nickname>::initialize(chatterino::SignalVector<chatterino::Nickname>*)::{lambda(auto:1)#1}&, chatterino::SignalVectorItemEvent<chatterino::Nickname>&&) (__fn=...) at /usr/include/c++/14.2.1/bits/invoke.h:111
#21 0x0000555555e658ec in std::_Function_handler<void (chatterino::SignalVectorItemEvent<chatterino::Nickname>), chatterino::SignalVectorModel<chatterino::Nickname>::initialize(chatterino::SignalVector<chatterino::Nickname>*)::{lambda(auto:1)#1}>::_M_invoke(std::_Any_data const&, chatterino::SignalVectorItemEvent<chatterino::Nickname>&&) (__functor=..., __args#0=...) at /usr/include/c++/14.2.1/bits/std_function.h:290
#22 0x00005555557ec19e in std::function<void(chatterino::SignalVectorItemEvent<chatterino::Nickname>)>::operator() (this=0x55555a8494f0, __args#0=...) at /usr/include/c++/14.2.1/bits/std_function.h:591
#23 0x00005555557eb436 in pajlada::Signals::Signal<chatterino::SignalVectorItemEvent<chatterino::Nickname> >::invoke (this=0x7fffffffd1f0, args#0=...) at :/lib/signals/include/pajlada/signals/signal.hpp:40
#24 0x00005555557eae0a in chatterino::SignalVector<chatterino::Nickname>::removeAt (this=0x7fffffffd1b0, index=1, caller=0x0) at :/src/common/SignalVector.hpp:125
#25 0x00005555557ea629 in chatterino::SignalVectorModel<chatterino::Nickname>::removeRows (this=0x7fffe4002740, row=1, count=1, parent=...) at :/src/common/SignalVectorModel.hpp:300
#26 0x0000555555dbc7f4 in QAbstractItemModel::removeRow (this=0x7fffe4002740, arow=1, aparent=...) at /usr/include/qt/QtCore/qabstractitemmodel.h:373
#27 0x0000555555dba744 in operator() (__closure=0x55555b5876c0) at :/src/widgets/helper/EditableModelView.cpp:67
#28 0x0000555555dbc75a in QtPrivate::FunctorCall<QtPrivate::IndexesList<>, QtPrivate::List<>, void, chatterino::EditableModelView::EditableModelView(QAbstractTableModel*, bool)::<lambda()> >::call(struct {...} &, void **) (f=..., arg=0x7fffffff4b00) at /usr/include/qt/QtCore/qobjectdefs_impl.h:146
#29 0x0000555555dbc50b in QtPrivate::Functor<chatterino::EditableModelView::EditableModelView(QAbstractTableModel*, bool)::<lambda()>, 0>::call<QtPrivate::List<>, void>(struct {...} &, void *, void **) (f=..., arg=0x7fffffff4b00) at /usr/include/qt/QtCore/qobjectdefs_impl.h:256
#30 0x0000555555dbc408 in QtPrivate::QFunctorSlotObject<chatterino::EditableModelView::EditableModelView(QAbstractTableModel*, bool)::<lambda()>, 0, QtPrivate::List<>, void>::impl(int, QtPrivate::QSlotObjectBase *, QObject *, void **, bool *)
    (which=1, this_=0x55555b5876b0, r=0x55555b5869e0, a=0x7fffffff4b00, ret=0x0) at /usr/include/qt/QtCore/qobjectdefs_impl.h:443
#31 0x00007ffff6cdfa9e in ??? () at /usr/lib/libQt5Core.so.5
#32 0x00007ffff7a441a5 in QAbstractButton::clicked(bool) () at /usr/lib/libQt5Widgets.so.5
#33 0x00007ffff7a46b73 in ??? () at /usr/lib/libQt5Widgets.so.5
#34 0x00007ffff7a48101 in ??? () at /usr/lib/libQt5Widgets.so.5
#35 0x00007ffff6cd1cc9 in QObject::event(QEvent*) () at /usr/lib/libQt5Core.so.5
#36 0x00007ffff7956331 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /usr/lib/libQt5Widgets.so.5
#37 0x00007ffff6caba68 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /usr/lib/libQt5Core.so.5
#38 0x00007ffff6cf9a80 in QTimerInfoList::activateTimers() () at /usr/lib/libQt5Core.so.5
#39 0x00007ffff6cfa202 in ??? () at /usr/lib/libQt5Core.so.5
#40 0x00007ffff53a3ab9 in ??? () at /usr/lib/libglib-2.0.so.0
#41 0x00007ffff54059e7 in ??? () at /usr/lib/libglib-2.0.so.0
#42 0x00007ffff53a2fc5 in g_main_context_iteration () at /usr/lib/libglib-2.0.so.0
#43 0x00007ffff6cfa37f in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib/libQt5Core.so.5
#44 0x00007ffff6ca382c in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib/libQt5Core.so.5
#45 0x00007ffff6cafbfd in QCoreApplication::exec() () at /usr/lib/libQt5Core.so.5
#46 0x00005555556107b0 in chatterino::Application::run (this=0x7fffffff50c0) at :/src/Application.cpp:307
#47 0x0000555555645358 in chatterino::runGui (a=..., paths=..., settings=..., args=..., updates=...) at :/src/RunGui.cpp:266
#48 0x00005555555fa94b in main (argc=1, argv=0x7fffffffd698) at :/src/main.cpp:122
(gdb)

[Nerixyz]

I don't really have the time to fully diagnose and fix this but here is the stack trace. This is a similar but not exactly the same crash, Arch, built on the test branch. This was removing a Nickname while editing one.
Repo root path was long so I replaced it with :/.

This can be fixed by the following patch:

diff --git a/src/common/SignalVectorModel.hpp b/src/common/SignalVectorModel.hpp
index 620ca452d..f04401438 100644
--- a/src/common/SignalVectorModel.hpp
+++ b/src/common/SignalVectorModel.hpp
@@ -167,7 +167,7 @@ public:
             int vecRow = this->getVectorIndexFromModelIndex(row);
             // TODO: This is only a safety-thing for when we modify data that's being modified right now.
             // It should not be necessary, but it would require some rethinking about this surrounding logic
-            if (vecRow >= this->vector_->readOnly()->size())
+            if (vecRow >= this->vector_->raw().size())
             {
                 return false;
             }

Basic issue is that SignalVector invokes itemRemoved before it updates readOnly when removing an item. In this case, itemRemoved eventually causes a call to SignalVectorModel::setData which checks the readOnly version. As the comment above explains, this isn't a permanent solution.

chatterino2/src/common/SignalVector.hpp

Lines 124 to 127 in 6e104bd

	SignalVectorItemEvent<T> args{item, index, caller};
	this->itemRemoved.invoke(args);
	this->itemsChanged_();