Bug 507547 - Failed assertion when inserting a new subtitle line above the first.
Summary: Failed assertion when inserting a new subtitle line above the first.
Status: RESOLVED FIXED
Alias: None
Product: subtitlecomposer
Classification: Applications
Component: general (other bugs)
Version First Reported In: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Mladen Milinkovic, Max
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-07-27 18:13 UTC by Matthias Grimrath
Modified: 2025-08-01 07:14 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
debug output in ObjectRef<> class (1.11 KB, text/plain)
2025-07-29 12:36 UTC, Matthias Grimrath
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Matthias Grimrath 2025-07-27 18:13:34 UTC
STEPS TO REPRODUCE
- build subtitlecomposer with debug and assertions enabled
- open a subtitle file
- select the first subtitle line
- select command "insert before" from the context menu

OBSERVED RESULT
- failed assertion: 'm_ref' is null, but it shouldn't

According to C++ specs, in release builds, program behaviour after the failed assertion is undefined, though in practice it is not.

SOFTWARE/OS VERSIONS
- shouldn't matter

ADDITIONAL INFORMATION

I already tracked it down to the method "SubtitleLine::index()" in "src/core/subtitleline.cpp". Apparently, there is some convoluted template magic with Qt going on, which may not call the constructor of helper objects as one would expect. It also doesn't seem to be first time this issue has popped up, because there is already some code there to work around this issue.

I already patched it for myself. If you are interested, what is the proper way to make this patch available? I heard of pull requests, but I have no experience how those works.
Comment 1 Mladen Milinkovic, Max 2025-07-28 08:10:54 UTC
Thanks for the report. I cannot reproduce it here.

> - failed assertion: 'm_ref' is null, but it shouldn't
I would appreciate exact message that is written - it should says something like:
ASSERT: "zzzzzzzz" in file xxxxxxx/xxxxx.cpp, line yyy

> According to C++ specs, in release builds, program behaviour after the failed assertion is undefined, though in practice it is not.
According to C++ specs?

> SOFTWARE/OS VERSIONS
> - shouldn't matter
It shouldn't, but please:
- What OS and version are you using?
- What compiler and version are you using? Is it one shipped with that OS/distribution?
- What Qt version are you using? Is it one shipped with that OS/distribution?

If the assert says:
ASSERT: "m_ref != nullptr" in file ........./subtitleline.cpp, line 230
your compiler is optimizing too much away and not calling object constructors when resizing vectors.

I saw this happen in some old Linux Mint build worker, but was unable to reproduce it anywhere.
Comment 2 Matthias Grimrath 2025-07-28 11:51:49 UTC
(In reply to Mladen Milinkovic, Max from comment #1)
> I would appreciate exact message that is written - it should says something like:
ASSERT: "m_ref != nullptr" in file /home/matthias/build/subtitlecomposer-upstream/src/core/subtitleline.cpp, line 230

Build from this source:

branch 'master' of https://invent.kde.org/multimedia/subtitlecomposer

commit a5a9b4916787f3b5263945cd63840fcb8e5a43cf (HEAD -> master, origin/master, origin/HEAD)
Author: l10n daemon script <scripty@kde.org>
Date:   Wed Jul 16 03:01:03 2025 +0000

    GIT_SILENT Sync po/docbooks with svn

> According to C++ specs?

This is the code in question:

Q_ASSERT(m_ref != nullptr);
Q_ASSERT(m_ref->m_obj == this);

const int index = m_ref - m_subtitle->m_lines.constData();

if(index < 0 || index >= m_subtitle->count()) { ...

So if 'm_ref' is NULL, 'index' contains garbage. Maybe it's not undefined by specification - I'm no language lawyer :) - but nevertheless I think it is not desirable. In practice though the bogus value in 'index' probably triggers the following 'if' clause, so the still-in-place work-around code amends the situation.

> > SOFTWARE/OS VERSIONS
> It shouldn't, but please:
> - What OS and version are you using?

Artix Linux, Arch derivative, a rolling release distribution

> - What compiler and version are you using? Is it one shipped with that OS/distribution?
yes, from the distribution
$ g++ -v
Using built-in specs.
COLLECT_GCC=g++
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-pc-linux-gnu/15.1.1/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /build/gcc/src/gcc/configure --enable-languages=ada,c,c++,d,fortran,go,lto,m2,objc,obj-c++,rust,cobol --enable-bootstrap --prefix=/usr --libdir=/usr/lib --libexecdir=/usr/lib --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=https://gitea.artixlinux.org/packages/gcc/issues --with-build-config=bootstrap-lto --with-linker-hash-style=gnu --with-system-zlib --enable-__cxa_atexit --enable-cet=auto --enable-checking=release --enable-clocale=gnu --enable-default-pie --enable-default-ssp --enable-gnu-indirect-function --enable-gnu-unique-object --enable-libstdcxx-backtrace --enable-link-serialization=1 --enable-linker-build-id --enable-lto --enable-multilib --enable-plugin --enable-shared --enable-threads=posix --disable-libssp --disable-libstdcxx-pch --disable-werror
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 15.1.1 20250425 (GCC)

> - What Qt version are you using? Is it one shipped with that OS/distribution?
Yes, pacman says 'local/qt6-base 6.9.0-1 (qt6)'

> If the assert says:
> ASSERT: "m_ref != nullptr" in file ........./subtitleline.cpp, line 230
> your compiler is optimizing too much away and not calling object
> constructors when resizing vectors.
> 
> I saw this happen in some old Linux Mint build worker, but was unable to
> reproduce it anywhere.

In my experience C++ (not only) template semantics are very tricky. So I wouldn't be surprised if the language spec allows that constructors in this case here (Qt Linesmodel) may not be called as one would expect.

For this same reason (tricky semantics) I try to avoid using own non-trivial classes with STL containers.
Comment 3 Matthias Grimrath 2025-07-28 11:52:49 UTC
# backtrace of failed assertion
(gdb) bt
#0  0x00007ffff0ea5ddc in ?? () from /usr/lib/libc.so.6
#1  0x00007ffff0e4f4d8 in raise () from /usr/lib/libc.so.6
#2  0x00007ffff0e37669 in abort () from /usr/lib/libc.so.6
#3  0x00007ffff14911dc in ?? () from /usr/lib/libQt6Core.so.6
#4  0x00007ffff14920ee in QMessageLogger::fatal(char const*, ...) const () from /usr/lib/libQt6Core.so.6
#5  0x00007ffff148f7d6 in qt_assert(char const*, char const*, int) () from /usr/lib/libQt6Core.so.6
#6  0x00005555555f941f in SubtitleComposer::SubtitleLine::index (this=0x5555592b3320) at /home/matthias/build/subtitlecomposer-upstream/src/core/subtitleline.cpp:230
#7  0x000055555565fd32 in SubtitleComposer::LinesModel::onModelReset (this=this@entry=0x555555fa7b60)
    at /home/matthias/build/subtitlecomposer-upstream/src/gui/treeview/linesmodel.cpp:339
#8  0x0000555555617430 in SubtitleComposer::LinesModel::processSelectionUpdate (this=0x555555fa7b60)
    at /home/matthias/build/subtitlecomposer-upstream/src/gui/treeview/linesmodel.h:51
#9  0x0000555555616d7d in saveSelection (current=0x555555bd7e0c, selection=0x555555bd7e28) at /home/matthias/build/subtitlecomposer-upstream/src/core/undo/undostack.cpp:81
#10 0x0000555555616e41 in SubtitleComposer::UndoStack::levelDecrease (this=this@entry=0x555556102790, idx=idx@entry=1)
    at /home/matthias/build/subtitlecomposer-upstream/src/core/undo/undostack.cpp:109
#11 0x00005555556173ae in SubtitleComposer::UndoStack::push (this=0x555556102790, cmd=0x5555594117d0)
    at /home/matthias/build/subtitlecomposer-upstream/src/core/undo/undostack.cpp:121
#12 0x00005555555eb0b8 in SubtitleComposer::Subtitle::processAction (this=this@entry=0x555559232ca0, action=action@entry=0x5555594117d0)
    at /home/matthias/build/subtitlecomposer-upstream/src/core/subtitle.cpp:1390
#13 0x00005555555ebd07 in SubtitleComposer::Subtitle::insertLine (this=this@entry=0x555559232ca0, line=line@entry=0x5555592b3320, index=index@entry=0)
    at /home/matthias/build/subtitlecomposer-upstream/src/core/subtitle.cpp:361
#14 0x00005555555ecb8e in SubtitleComposer::Subtitle::insertNewLine (this=this@entry=0x555559232ca0, index=0, insertAfter=insertAfter@entry=false,
    target=target@entry=SubtitleComposer::Both) at /home/matthias/build/subtitlecomposer-upstream/src/core/subtitle.cpp:400
#15 0x00005555555bbc37 in SubtitleComposer::Application::insertBeforeCurrentLine (this=0x7fffffffdd30) at /home/matthias/build/subtitlecomposer-upstream/src/application.cpp:420
#16 0x00005555555c60ee in QtPrivate::FunctorCall<std::integer_sequence<unsigned long>, QtPrivate::List<>, void, void (SubtitleComposer::Application::*)()>::call(void (SubtitleComposer::Application::*)(), SubtitleComposer::Application*, void**)::{lambda()#1}::operator()() const (__closure=__closure@entry=0x7fffffffb8c0)
    at /usr/include/qt6/QtCore/qobjectdefs_impl.h:127
#17 0x00005555555c709c in QtPrivate::FunctorCallBase::call_internal<void, QtPrivate::FunctorCall<std::integer_sequence<unsigned long>, QtPrivate::List<>, void, void (SubtitleComposer::Application::*)()>::call(void (SubtitleComposer::Application::*)(), SubtitleComposer::Application*, void**)::{lambda()#1}>(void**, QtPrivate::FunctorCall<std::integer_sequence<unsigned long>, QtPrivate::List<>, void, void (SubtitleComposer::Application::*)()>::call(void (SubtitleComposer::Application::*)(), SubtitleComposer::Application*, void**)::{lambda()#1}&&) (args=<optimized out>, fn=...) at /usr/include/qt6/QtCore/qobjectdefs_impl.h:65
#18 QtPrivate::FunctorCall<std::integer_sequence<unsigned long>, QtPrivate::List<>, void, void (SubtitleComposer::Application::*)()>::call(void (SubtitleComposer::Application::*)(), SubtitleComposer::Application*, void**) (
    f=(void (SubtitleComposer::Application::*)(SubtitleComposer::Application * const)) 0x5555555bbb52 <SubtitleComposer::Application::insertBeforeCurrentLine()>,
    o=<optimized out>, arg=<optimized out>) at /usr/include/qt6/QtCore/qobjectdefs_impl.h:126
#19 0x00005555555c70ee in QtPrivate::FunctionPointer<void (SubtitleComposer::Application::*)()>::call<QtPrivate::List<>, void>(void (SubtitleComposer::Application::*)(), SubtitleComposer::Application*, void**) (f=<optimized out>, o=<optimized out>, arg=<optimized out>) at /usr/include/qt6/QtCore/qobjectdefs_impl.h:174
#20 QtPrivate::QCallableObject<void (SubtitleComposer::Application::*)(), QtPrivate::List<>, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (
    which=<optimized out>, this_=<optimized out>, r=<optimized out>, a=<optimized out>, ret=<optimized out>) at /usr/include/qt6/QtCore/qobjectdefs_impl.h:545
#21 0x00007ffff15b2dba in ?? () from /usr/lib/libQt6Core.so.6
#22 0x00007ffff2164f99 in QAction::activate(QAction::ActionEvent) () from /usr/lib/libQt6Gui.so.6
#23 0x00007ffff28d809a in ?? () from /usr/lib/libQt6Widgets.so.6
#24 0x00007ffff28dce99 in ?? () from /usr/lib/libQt6Widgets.so.6
#25 0x00007ffff274eecf in QWidget::event(QEvent*) () from /usr/lib/libQt6Widgets.so.6
#26 0x00007ffff26fea7b in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/libQt6Widgets.so.6
#27 0x00007ffff2702803 in QApplication::notify(QObject*, QEvent*) () from /usr/lib/libQt6Widgets.so.6
#28 0x00007ffff1557f48 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /usr/lib/libQt6Core.so.6
#29 0x00007ffff26f6833 in QApplicationPrivate::sendMouseEvent(QWidget*, QMouseEvent*, QWidget*, QWidget*, QWidget**, QPointer<QWidget>&, bool, bool) ()
   from /usr/lib/libQt6Widgets.so.6
#30 0x00007ffff27695eb in ?? () from /usr/lib/libQt6Widgets.so.6
#31 0x00007ffff276a520 in ?? () from /usr/lib/libQt6Widgets.so.6
#32 0x00007ffff26fea7b in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/libQt6Widgets.so.6
#33 0x00007ffff1557f48 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /usr/lib/libQt6Core.so.6
#34 0x00007ffff1d88a45 in QGuiApplicationPrivate::processMouseEvent(QWindowSystemInterfacePrivate::MouseEvent*) () from /usr/lib/libQt6Gui.so.6
#35 0x00007ffff1e081d4 in QWindowSystemInterface::sendWindowSystemEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQt6Gui.so.6
#36 0x00007fffdfa26c8f in ?? () from /usr/lib/qt6/plugins/platforms/../../../libQt6XcbQpa.so.6
#37 0x00007fffef702c1f in ?? () from /usr/lib/libglib-2.0.so.0
#38 0x00007fffef764337 in ?? () from /usr/lib/libglib-2.0.so.0
--Type <RET> for more, q to quit, c to continue without paging--c
#39 0x00007fffef7020d2 in g_main_context_iteration () from /usr/lib/libglib-2.0.so.0
#40 0x00007ffff17bf7a8 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQt6Core.so.6
#41 0x00007ffff1562f95 in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQt6Core.so.6
#42 0x00007ffff28e0178 in ?? () from /usr/lib/libQt6Widgets.so.6
#43 0x00007ffff28e02a0 in QMenu::exec(QPoint const&, QAction*) () from /usr/lib/libQt6Widgets.so.6
#44 0x0000555555662aca in SubtitleComposer::LinesWidget::contextMenuEvent (this=0x555555f98a60, e=0x7fffffffcfc0)
    at /home/matthias/build/subtitlecomposer-upstream/src/gui/treeview/lineswidget.cpp:439
#45 0x00007ffff274f16a in QWidget::event(QEvent*) () from /usr/lib/libQt6Widgets.so.6
#46 0x00007ffff27eb224 in QFrame::event(QEvent*) () from /usr/lib/libQt6Widgets.so.6
#47 0x00007ffff1557268 in QCoreApplicationPrivate::sendThroughObjectEventFilters(QObject*, QEvent*) () from /usr/lib/libQt6Core.so.6
#48 0x00007ffff26fea6b in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/libQt6Widgets.so.6
#49 0x00007ffff2703a34 in QApplication::notify(QObject*, QEvent*) () from /usr/lib/libQt6Widgets.so.6
#50 0x00007ffff1557f48 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /usr/lib/libQt6Core.so.6
#51 0x00007ffff276ac42 in ?? () from /usr/lib/libQt6Widgets.so.6
#52 0x00007ffff26fea7b in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/libQt6Widgets.so.6
#53 0x00007ffff1557f48 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /usr/lib/libQt6Core.so.6
#54 0x00007ffff1df0fd6 in QWindowPrivate::maybeSynthesizeContextMenuEvent(QMouseEvent*) () from /usr/lib/libQt6Gui.so.6
#55 0x00007ffff27692fc in ?? () from /usr/lib/libQt6Widgets.so.6
#56 0x00007ffff276a520 in ?? () from /usr/lib/libQt6Widgets.so.6
#57 0x00007ffff26fea7b in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/libQt6Widgets.so.6
#58 0x00007ffff1557f48 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /usr/lib/libQt6Core.so.6
#59 0x00007ffff1d88a45 in QGuiApplicationPrivate::processMouseEvent(QWindowSystemInterfacePrivate::MouseEvent*) () from /usr/lib/libQt6Gui.so.6
#60 0x00007ffff1e081d4 in QWindowSystemInterface::sendWindowSystemEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQt6Gui.so.6
#61 0x00007fffdfa26c8f in ?? () from /usr/lib/qt6/plugins/platforms/../../../libQt6XcbQpa.so.6
#62 0x00007fffef702c1f in ?? () from /usr/lib/libglib-2.0.so.0
#63 0x00007fffef764337 in ?? () from /usr/lib/libglib-2.0.so.0
#64 0x00007fffef7020d2 in g_main_context_iteration () from /usr/lib/libglib-2.0.so.0
#65 0x00007ffff17bf7a8 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQt6Core.so.6
#66 0x00007ffff1562f95 in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQt6Core.so.6
#67 0x00007ffff155af14 in QCoreApplication::exec() () from /usr/lib/libQt6Core.so.6
#68 0x00005555555b9d36 in main (argc=<optimized out>, argv=<optimized out>) at /home/matthias/build/subtitlecomposer-upstream/src/main.cpp:181
Comment 4 Matthias Grimrath 2025-07-28 11:54:53 UTC
How I build subtitle-composer

$ cat build.sh	# my script to configure and compile
cmake -B /tmp/build-subtitle -G Ninja -DQT_MAJOR_VERSION=6 -DCMAKE_BUILD_TYPE=Debug -DCMAKE_INSTALL_PREFIX=/tmp/inst-subtitle
ninja -C /tmp/build-subtitle -j 12
ninja -C /tmp/build-subtitle install

# excerpts from build log
$ touch src/core/subtitleline.cpp
$ ninja -v -C /tmp/build-subtitle/
[4/14] /usr/bin/c++ -DKCOREADDONS_LIB -DKGUIADDONS_LIB -DQT_CORE5COMPAT_LIB -DQT_CORE_LIB -DQT_DBUS_LIB -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_OPENGLWIDGETS_LIB -DQT_OPENGL_LIB -DQT_QMLINTEGRATION_LIB -DQT_QML_LIB -DQT_WIDGETS_LIB -DQT_XML_LIB -D_DEFAULT_SOURCE -D_GNU_SOURCE -D_LARGEFILE64_SOURCE -I/tmp/build-subtitle/src -I/home/matthias/build/subtitlecomposer-upstream/src -I/tmp/build-subtitle/src/subtitlecomposer-lib_autogen/include -I/home/matthias/build/subtitlecomposer-upstream/src/widgets -I/usr/include/qt6/QtDBus/6.9.0 -I/usr/include/qt6/QtDBus/6.9.0/QtDBus -I/usr/include/AL -isystem /usr/include/qt6/QtCore -isystem /usr/include/qt6 -isystem /usr/include/qt6/QtWidgets -isystem /usr/include/qt6/QtGui -isystem /usr/include/qt6/QtDBus -isystem /usr/include/qt6/QtWidgets/6.9.0 -isystem /usr/include/qt6/QtWidgets/6.9.0/QtWidgets -isystem /usr/include/qt6/QtCore/6.9.0 -isystem /usr/include/qt6/QtCore/6.9.0/QtCore -isystem /usr/include/qt6/QtGui/6.9.0 -isystem /usr/include/qt6/QtGui/6.9.0/QtGui -isystem /usr/lib/qt6/mkspecs/linux-g++ -isystem /usr/include/qt6/QtQml -isystem /usr/include/qt6/QtQmlIntegration -isystem /usr/include/qt6/QtNetwork -isystem /usr/include/KF6/KCoreAddons -isystem /usr/include/KF6/KWidgetsAddons -isystem /usr/include/KF6/KTextWidgets -isystem /usr/include/KF6/SonnetUi -isystem /usr/include/KF6/Sonnet -isystem /usr/include/KF6/KI18n -isystem /usr/include/KF6/KCodecs -isystem /usr/include/KF6/SonnetCore -isystem /usr/include/KF6/KIOCore -isystem /usr/include/KF6/KIO -isystem /usr/include/KF6/KIOFileWidgets -isystem /usr/include/KF6/KIOWidgets -isystem /usr/include/KF6/KIOGui -isystem /usr/include/KF6/KConfig -isystem /usr/include/KF6/KConfigCore -isystem /usr/include/KF6/KService -isystem /usr/include/KF6/KJobWidgets -isystem /usr/include/KF6/Solid -isystem /usr/include/KF6/KCompletion -isystem /usr/include/KF6/KBookmarks -isystem /usr/include/qt6/QtXml -isystem /usr/include/KF6/KItemViews -isystem /usr/include/KF6/KXmlGui -isystem /usr/include/KF6/KConfigWidgets -isystem /usr/include/KF6/KConfigGui -isystem /usr/include/KF6/KColorScheme -isystem /usr/include/KF6/KGuiAddons -isystem /usr/include/qt6/QtOpenGLWidgets -isystem /usr/include/qt6/QtOpenGL -isystem /usr/include/qt6/QtCore5Compat -fno-operator-names -fno-exceptions -Wall -Wextra -Wcast-align -Wchar-subscripts -Wformat-security -Wno-long-long -Wpointer-arith -Wundef -Wnon-virtual-dtor -Woverloaded-virtual -Werror=return-type -Werror=init-self -Wvla -Wdate-time -Wsuggest-override -Wlogical-op -fdiagnostics-color=always -g -Wall -Og -g -std=gnu++17 -fvisibility=hidden -fvisibility-inlines-hidden -mno-direct-extern-access -MD -MT src/CMakeFiles/subtitlecomposer-lib.dir/core/subtitleline.cpp.o -MF src/CMakeFiles/subtitlecomposer-lib.dir/core/subtitleline.cpp.o.d -o src/CMakeFiles/subtitlecomposer-lib.dir/core/subtitleline.cpp.o -c /home/matthias/build/subtitlecomposer-upstream/src/core/subtitleline.cpp
Comment 5 Mladen Milinkovic, Max 2025-07-28 15:11:30 UTC
(In reply to Matthias Grimrath from comment #2)
> (In reply to Mladen Milinkovic, Max from comment #1)
> > I would appreciate exact message that is written - it should says something like:
> ASSERT: "m_ref != nullptr" in file
> /home/matthias/build/subtitlecomposer-upstream/src/core/subtitleline.cpp,
> line 230

As I said before I think your compiler is optimizing too much code.
I have done exact same test on current arch linux and cannot reproduce the problem.

QVector when growing copies the existing values into newly allocated bigger buffer.
When vector contains C++ objects it's supposed to copy/move construct them depending if they are *NOT* Q_MOVABLE_TYPE.
If they are Q_MOVABLE_TYPE QVector can just memcpy old data to new buffer without calling any constructors.

Since ObjectRef is not Q_MOVABLE_TYPE, QVector is supposed to call C++ constructor when growing - which in turns assign m_ref to something non-null on newly created ObjectRef instances.

Your build triggering Q_ASSERT(m_ref != nullptr) means that for whatever reason QVector when growing is not constructing C++ objects, but just memcpy-ing their memory.
I'm not sure why is that happening, if you feel like investigating you could try building Qt yourself and checking what's going on in qvector.cpp (you will see Q_MOVABLE_TYPE if conditions for optimizations).

In any case if you just disable asserts (or remove those two asserts) the code will continue working as it was and it will log that qWarning whenever it it's uynable to correctly calculate index of the element. In that case it will fallback to original for loop that was used to figure out the subtitle line index.
Comment 6 Matthias Grimrath 2025-07-28 17:46:19 UTC
I think I found the found problem, but I still need to work out the details and write it up. I don't like to say it, but it looks like there is indeed a deep flaw (as far as C++ semantics are concerned) in the 'ObjectRef' template class used inside a 'QVector<>'.

(In reply to Mladen Milinkovic, Max from comment #5)
> As I said before I think your compiler is optimizing too much code.
> I have done exact same test on current arch linux and cannot reproduce the
> problem.

I think what triggers the assertion on my computer is not too much optimization, but the lack thereof. Did you configure and build subtitle-composer in a debug build as I did? As I wrote above, I invoked 'cmake' with build type set to 'Debug' and recompiled from scratch without optimizations.

$ cmake -DQT_MAJOR_VERSION=6 -DCMAKE_BUILD_TYPE=Debug
Comment 7 Matthias Grimrath 2025-07-29 12:36:54 UTC
Created attachment 183632 [details]
debug output in ObjectRef<> class
Comment 8 Matthias Grimrath 2025-07-29 12:38:27 UTC
I added some additional debug output (see attached patch) which reveals what triggers the assertion on my computer.

The 'ObjectRef<>' class checks wether the pointer points inside the QVector<> array, and if not, leaves 'm_ref' at its NULL value. So added I some code to print the pointer values if a call to 'ref()' or 'movref()' skips assigning to 'm_ref' because of that.

this=0x555555f500b0 beg=0x555555f500c0 end=0x555555f502b0 cap=31 len=9
this=0x555555f500b0 m_obj=0x555559314e80 ignoring
ASSERT: "m_ref != nullptr" in file /home/matthias/build/subtitlecomposer-upstream/src/core/subtitleline.cpp, line 230

What is odd is that the 'this' value (0x555555f500b0) is exactly one element before the beginning of the QVector<> array (0x555555f500c0). So 'm_ref' is left at its NULL value, although it suspiciously looks like it should be a member of the 'QVector<>' object.

If a set a breakpoint at the code where it decides not to set 'm_ref'

(gdb) b ObjectRef<SubtitleComposer::SubtitleLine>::inContainerXXX()

I get this backtrace:

(gdb) bt
#0  SubtitleComposer::ObjectRef<SubtitleComposer::SubtitleLine>::inContainerXXX (this=this@entry=0x7fffffffb4c0)
    at /home/matthias/build/subtitlecomposer-upstream/src/helpers/objectref.h:82
#1  0x00005555555c5e0a in SubtitleComposer::ObjectRef<SubtitleComposer::SubtitleLine>::moveref (this=this@entry=0x7fffffffb4c0, other=other@entry=0x7fffffffb580)
    at /home/matthias/build/subtitlecomposer-upstream/src/helpers/objectref.h:62
#2  0x0000555555615df8 in SubtitleComposer::ObjectRef<SubtitleComposer::SubtitleLine>::ObjectRef (this=0x7fffffffb4c0, other=...)
    at /home/matthias/build/subtitlecomposer-upstream/src/helpers/objectref.h:35
#3  QtPrivate::QGenericArrayOps<SubtitleComposer::ObjectRef<SubtitleComposer::SubtitleLine> >::emplace<SubtitleComposer::ObjectRef<SubtitleComposer::SubtitleLine> > (this=this@entry=0x5555561fc090, i=i@entry=0) at /usr/include/qt6/QtCore/qarraydataops.h:545

The code at qarraydataops.h:545 reveals what happens here:

        if (growsAtBegin) {
            Q_ASSERT(this->freeSpaceAtBegin());
            new (this->begin() - 1) T(std::move(tmp));
            --this->ptr;
            ++this->size;
        } else {

So QVector<> *does* call the constructor of ObjectRef<>, but it hasn't updated its internal pointers yet. So when 'inContainer()' checks the pointer, it gets outdated values from the QVector<> object and thus wrongly assumes the 'SubtitleLine' pointer is outside the array.
Comment 9 Mladen Milinkovic, Max 2025-07-29 20:02:06 UTC
Apologies I was building with wrong Qt version instead of system's 6.9.1.
I have managed to reproduce it here.
Comment 10 Mladen Milinkovic, Max 2025-07-30 17:28:12 UTC
Have changed ObjectRef and m_lines to use std::vector with custom allocator, so that the code doesn't rely anymore on QVector's internal implementation.

Still haven't pushed it to master as I want to test and clean it up some more.
You can checkout work/maxrd2/vector branch if you'd like to try it out.
https://invent.kde.org/multimedia/subtitlecomposer/-/commits/work/maxrd2/vector?ref_type=heads
Comment 11 Matthias Grimrath 2025-07-31 17:31:15 UTC
You dare to go, were I wouldn't, by using allocators :D

This, however, looks suspicious to me

inline bool inContainer() {
	const auto &a = container()->get_allocator();
	return this >= a.lastData && this < a.lastData + a.lastSize;

so the pointer is compared against the allocated memory. However, a pointer somewhere into that allocated memory may not point to an actually used element. We saw QVector<> reserving some space at the beginning to be able to quickly insert elements at the top. And a STL implementation of std::vector<> is probably allowed to do the same.

I don't know weather that leads to real-world problems. But I feel uneasy about it.

I think a better approach would be to control insertion of new elements into 'm_lines' and update indices there, instead of hooking into QVector<> or std::vector<> by template magic. Should be the same performance wise.

I'll work on a patch, having real code to show is probably better than sophisticated arguments :D
Comment 12 Mladen Milinkovic, Max 2025-07-31 18:46:05 UTC
(In reply to Matthias Grimrath from comment #11)
> You dare to go, were I wouldn't, by using allocators :D
> 
> This, however, looks suspicious to me
> 
> inline bool inContainer() {
> 	const auto &a = container()->get_allocator();
> 	return this >= a.lastData && this < a.lastData + a.lastSize;
> 
> so the pointer is compared against the allocated memory. However, a pointer
> somewhere into that allocated memory may not point to an actually used
> element. We saw QVector<> reserving some space at the beginning to be able
> to quickly insert elements at the top. And a STL implementation of
> std::vector<> is probably allowed to do the same.
As long as memory is allocated to that container it's fine IMHO, regardless if it's reserved or used.
If something is moving object into memory allocated by m_lines it's about to become or already is member of m_lines.

> I don't know weather that leads to real-world problems. But I feel uneasy
> about it.
> 
> I think a better approach would be to control insertion of new elements into
> 'm_lines' and update indices there, instead of hooking into QVector<> or
> std::vector<> by template magic. Should be the same performance wise.
It's not magic - just code :D - and this one doesn't rely on unspecified internal implementation of std::vector. Previous version relied on QVector constructing elements in memory between begin() and end() (it had to update begin/end() *before* move/copy/constructing element object).

If you want to do PR it's fine by me - although keep in mind that similar approach was used before and it wasn't working well. At times when subtitle or undo stack handling code was touched it often caused subtle bugs and it was maintenance nightmare. Sorting performance was awful due to index recalculation every time lines would get swapped and so on. :)

> I'll work on a patch, having real code to show is probably better than
> sophisticated arguments :D
The safest (code wise) and best (performance wise) would be to use custom array class and handle memory allocation directly, but I'm quite happy with custom allocator like it is now.
Comment 13 Mladen Milinkovic, Max 2025-08-01 07:14:03 UTC
Have merged the fix to the master branch.
Feel free to re-open this issue if you still have problems.
Also you can open MR on https://invent.kde.org/multimedia/subtitlecomposer if you'd like to provide better solution.

Thanks for the report and your time.