Bug 433347

Hi, we would need debug symbol for the backtrace to be understandable.

I am not able to reproduce the problem (on a different system).

Operating System: Debian GNU/Linux
KDE Plasma Version: 5.20.5
KDE Frameworks Version: 5.78.0
Qt Version: 5.15.2
Kernel Version: 5.10.0-3-amd64
OS Type: 64-bit
Graphics Platform: X11

It seems that the problem is not "repeatedly opening and closing the New Directory dialog", but "opening more than one New Directory dialog".

A simpler way to reproduce it is:

1. Launch dolphin.
2. Smash the F10 button attempting to open several New Directory dialogs, especially while Dolphin has not finished loading the current directory yet.
3. Several New Directory dialogs will be open. Close the topmost one.
4. Attempting to edit text in any of the remaining New Directory dialogs will crash Dolphin.

When running under Valgrind, Dolphin is slow enough that you can open tenths of dialogs at once. But it also generates notifications like the following, even without pressing the "OK" button in the dialogs:

> Examining (Failed)
> /home/magiblot//New Folder

Valgring log:

> ==88948== Invalid read of size 8
> ==88948==    at 0x5D6003A: QLineEdit::text() const (qlineedit.cpp:312)
> ==88948==    by 0x4D026CC: KNewFileMenuPrivate::_k_slotStatResult(KJob*) (knewfilemenu.cpp:1231)
> ==88948==    by 0x6B0ED85: call (qobjectdefs_impl.h:398)
> ==88948==    by 0x6B0ED85: void doActivate<false>(QObject*, int, void**) (qobject.cpp:3886)
> ==88948==    by 0x58B1C5C: KJob::result(KJob*, KJob::QPrivateSignal) (moc_kjob.cpp:576)
> ==88948==    by 0x58B279B: KJob::finishJob(bool) (kjob.cpp:92)
> ==88948==    by 0x6B0ED85: call (qobjectdefs_impl.h:398)
> ==88948==    by 0x6B0ED85: void doActivate<false>(QObject*, int, void**) (qobject.cpp:3886)
> ==88948==    by 0x510C0B6: KIO::SlaveInterface::error(int, QString const&) (moc_slaveinterface.cpp:452)
> ==88948==    by 0x510E4E3: KIO::SlaveInterface::dispatch(int, QByteArray const&) (slaveinterface.cpp:180)
> ==88948==    by 0x510C819: KIO::SlaveInterface::dispatch() (slaveinterface.cpp:77)
> ==88948==    by 0x511031A: KIO::Slave::gotInput() (slave.cpp:394)
> ==88948==    by 0x6B0ED85: call (qobjectdefs_impl.h:398)
> ==88948==    by 0x6B0ED85: void doActivate<false>(QObject*, int, void**) (qobject.cpp:3886)
> ==88948==    by 0x6B04531: QObject::event(QEvent*) (qobject.cpp:1314)
> ==88948==    by 0x5C1A751: QApplicationPrivate::notify_helper(QObject*, QEvent*) (qapplication.cpp:3632)
> ==88948==    by 0x6AD7A29: QCoreApplication::notifyInternal2(QObject*, QEvent*) (qcoreapplication.cpp:1063)
> ==88948==    by 0x6ADA522: QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (qcoreapplication.cpp:1817)
> ==88948==    by 0x6B31053: postEventSourceDispatch(_GSource*, int (*)(void*), void*) (qeventdispatcher_glib.cpp:277)
> ==88948==    by 0x8BF3B83: g_main_context_dispatch (in /usr/lib/libglib-2.0.so.0.6600.7)
> ==88948==    by 0x8C47C20: ??? (in /usr/lib/libglib-2.0.so.0.6600.7)
> ==88948==    by 0x8BF23B0: g_main_context_iteration (in /usr/lib/libglib-2.0.so.0.6600.7)
> ==88948==    by 0x6B30690: QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (qeventdispatcher_glib.cpp:423)
> ==88948==    by 0x6AD63AB: QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) (qeventloop.cpp:232)
> ==88948==    by 0x6ADE843: QCoreApplication::exec() (qcoreapplication.cpp:1371)
> ==88948==    by 0x48C4F42: kdemain (main.cpp:222)
> ==88948==    by 0x499FB24: (below main) (in /usr/lib/libc-2.33.so)
> ==88948==  Address 0xfbf3d18 is 8 bytes inside a block of size 48 free'd
> ==88948==    at 0x484008B: operator delete(void*, unsigned long) (vg_replace_malloc.c:593)
> ==88948==    by 0x6B020BD: QObjectPrivate::deleteChildren() (qobject.cpp:2104)
> ==88948==    by 0x5C5704D: QWidget::~QWidget() (qwidget.cpp:1522)
> ==88948==    by 0x5E1C819: QDialog::~QDialog() (qdialog.cpp:426)
> ==88948==    by 0x6B042EF: QObject::event(QEvent*) (qobject.cpp:1301)
> ==88948==    by 0x5C1A751: QApplicationPrivate::notify_helper(QObject*, QEvent*) (qapplication.cpp:3632)
> ==88948==    by 0x6AD7A29: QCoreApplication::notifyInternal2(QObject*, QEvent*) (qcoreapplication.cpp:1063)
> ==88948==    by 0x6ADA522: QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (qcoreapplication.cpp:1817)
> ==88948==    by 0x6B31053: postEventSourceDispatch(_GSource*, int (*)(void*), void*) (qeventdispatcher_glib.cpp:277)
> ==88948==    by 0x8BF3B83: g_main_context_dispatch (in /usr/lib/libglib-2.0.so.0.6600.7)
> ==88948==    by 0x8C47C20: ??? (in /usr/lib/libglib-2.0.so.0.6600.7)
> ==88948==    by 0x8BF23B0: g_main_context_iteration (in /usr/lib/libglib-2.0.so.0.6600.7)
> ==88948==    by 0x6B30690: QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (qeventdispatcher_glib.cpp:423)
> ==88948==    by 0x6AD63AB: QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) (qeventloop.cpp:232)
> ==88948==    by 0x6ADE843: QCoreApplication::exec() (qcoreapplication.cpp:1371)
> ==88948==    by 0x48C4F42: kdemain (main.cpp:222)
> ==88948==    by 0x499FB24: (below main) (in /usr/lib/libc-2.33.so)
> ==88948==  Block was alloc'd at
> ==88948==    at 0x483EDEF: operator new(unsigned long) (vg_replace_malloc.c:342)
> ==88948==    by 0x4D01C7A: KNewFileMenuPrivate::initDialog() (knewfilemenu.cpp:446)
> ==88948==    by 0x4D04AC2: KNewFileMenuPrivate::showNewDirNameDlg(QString const&) (knewfilemenu.cpp:1414)
> ==88948==    by 0x6B0ED85: call (qobjectdefs_impl.h:398)
> ==88948==    by 0x6B0ED85: void doActivate<false>(QObject*, int, void**) (qobject.cpp:3886)
> ==88948==    by 0x58B1C5C: KJob::result(KJob*, KJob::QPrivateSignal) (moc_kjob.cpp:576)
> ==88948==    by 0x58B279B: KJob::finishJob(bool) (kjob.cpp:92)
> ==88948==    by 0x6B0ED85: call (qobjectdefs_impl.h:398)
> ==88948==    by 0x6B0ED85: void doActivate<false>(QObject*, int, void**) (qobject.cpp:3886)
> ==88948==    by 0x58B1C5C: KJob::result(KJob*, KJob::QPrivateSignal) (moc_kjob.cpp:576)
> ==88948==    by 0x58B279B: KJob::finishJob(bool) (kjob.cpp:92)
> ==88948==    by 0x6B0ED85: call (qobjectdefs_impl.h:398)
> ==88948==    by 0x6B0ED85: void doActivate<false>(QObject*, int, void**) (qobject.cpp:3886)
> ==88948==    by 0x510C0B6: KIO::SlaveInterface::error(int, QString const&) (moc_slaveinterface.cpp:452)
> ==88948==    by 0x510E4E3: KIO::SlaveInterface::dispatch(int, QByteArray const&) (slaveinterface.cpp:180)
> ==88948==    by 0x510C819: KIO::SlaveInterface::dispatch() (slaveinterface.cpp:77)
> ==88948==    by 0x511031A: KIO::Slave::gotInput() (slave.cpp:394)
> ==88948==    by 0x6B0ED85: call (qobjectdefs_impl.h:398)
> ==88948==    by 0x6B0ED85: void doActivate<false>(QObject*, int, void**) (qobject.cpp:3886)
> ==88948==    by 0x6B04531: QObject::event(QEvent*) (qobject.cpp:1314)
> ==88948==    by 0x5C1A751: QApplicationPrivate::notify_helper(QObject*, QEvent*) (qapplication.cpp:3632)
> ==88948==    by 0x6AD7A29: QCoreApplication::notifyInternal2(QObject*, QEvent*) (qcoreapplication.cpp:1063)
> ==88948==    by 0x6ADA522: QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (qcoreapplication.cpp:1817)
> ==88948==    by 0x6B31053: postEventSourceDispatch(_GSource*, int (*)(void*), void*) (qeventdispatcher_glib.cpp:277)
> ==88948==    by 0x8BF3B83: g_main_context_dispatch (in /usr/lib/libglib-2.0.so.0.6600.7)
> ==88948==    by 0x8C47C20: ??? (in /usr/lib/libglib-2.0.so.0.6600.7)
> ==88948==    by 0x8BF23B0: g_main_context_iteration (in /usr/lib/libglib-2.0.so.0.6600.7)
> ==88948==    by 0x6B30690: QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (qeventdispatcher_glib.cpp:423)
> ==88948==    by 0x6AD63AB: QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) (qeventloop.cpp:232)
> ==88948==    by 0x6ADE843: QCoreApplication::exec() (qcoreapplication.cpp:1371)
> ==88948==    by 0x48C4F42: kdemain (main.cpp:222)
> ==88948==    by 0x499FB24: (below main) (in /usr/lib/libc-2.33.so)
>

@Meven: is this caused by the recent changes to KNewFileMenu?

@Meven: is this caused by the recent changes to KNewFileMenu?

Probably.

https://invent.kde.org/frameworks/kio/-/merge_requests/375 should help here, since having multiple modal dialogs won't be possible.

I was able to reproduce after a while of F10 + Esc.

This happens when the statJob finishes while the KNewFileMenu is being destroyed and its QLineEdit has already been destroyed...

*** Bug 439303 has been marked as a duplicate of this bug. ***

> This happens when the statJob finishes while the KNewFileMenu is being destroyed and its QLineEdit has already been destroyed...

It's a bit more crazy even.

The easiest way to reproduce is add a sleep(1) to the ::stat of any KIO worker. Then simply hit F10 to trigger the dialog and Esc to close it before the collision-stat had a chance to finish. As such the impact of this bug purely depends on the performance of stat. Over slow networks its probably fairly easy to trigger.

The crash happens because of somewhat wonky life time management. Here are some pertinent lines:

>     m_fileDialog = new QDialog(m_parentWidget);
>     m_fileDialog->setAttribute(Qt::WA_DeleteOnClose);
> ...
>     m_lineEdit = new QLineEdit(m_fileDialog);
> ...
>         QObject::connect(job, &KJob::result, q, [this](KJob *job) {
>             _k_slotStatResult(job);
>         });

i.e. the dialog deletes when it closes, the lineedit is a child of the dialog, hence the lineedit gets deleted BUT the actual signals are all bound to the life-time of the KNewFileMenu instance which has zero relationship with the life time of the aforementioned objects.
In particular in dolphin's case the Menu doesn't get deleted at all, its life time spans the window's. So during the life time of dolphin the same Menu instance can spawn a million different dialogs.

What might make sense as a fix here is to split the dialog logic out of the menu class. As far as NewFileMenu is concerned it doesn't seem to really need access to any of the dialog "internals". All it needs is to make a dialog, the dialog then encapsulates the entire logic of preventing name collision and has a conclusive end result "yo, use this name" or "rejected" at which point the dialog can get deleted again along with all its internal connections. By not giving the Menu instance any access to the widgets of the dialog this entire type of problem goes away. It a) cannot access widgets past its life b) the connections inside the dialog get scoped to the ephemeral dialog and also can't exist past the widgets' life.

I'll move the bug to frameworks cause dolphin isn't at fault here, in fact it can't even do anything to fix it.

*** Bug 437179 has been marked as a duplicate of this bug. ***

*** Bug 437372 has been marked as a duplicate of this bug. ***

*** Bug 439609 has been marked as a duplicate of this bug. ***

*** Bug 433993 has been marked as a duplicate of this bug. ***

https://invent.kde.org/frameworks/kio/-/merge_requests/664

*** Bug 448115 has been marked as a duplicate of this bug. ***

*** Bug 448608 has been marked as a duplicate of this bug. ***

*** Bug 448534 has been marked as a duplicate of this bug. ***

*** Bug 447907 has been marked as a duplicate of this bug. ***

*** Bug 442552 has been marked as a duplicate of this bug. ***

*** Bug 448659 has been marked as a duplicate of this bug. ***

I was able to cause this by merely invoking one dialog, but subsequent attempts by me to reproduce the crash have failed.

*** Bug 449665 has been marked as a duplicate of this bug. ***

Git commit c6e7a06cd3dcb5f148b49a4a08e7c7c76790aabe by Nicolas Fella.
Committed on 06/02/2022 at 14:25.
Pushed by nicolasfella into branch 'master'.

[knewfilemenu] Bind stat job connection lifetime to dialog, not the whole menu

The stat job is asynchronous so it may end after the dialog is closed and deleted.

In the slot we are accessing children of the dialog which then can lead to a crash

Change the third parameter in the connect to the dialog instead of the menu to automatically disconnect the connection when the dialog is destroyed

Thanks to Harald Sitter for the investigation.

M  +5    -1    src/filewidgets/knewfilemenu.cpp

https://invent.kde.org/frameworks/kio/commit/c6e7a06cd3dcb5f148b49a4a08e7c7c76790aabe

*** Bug 449886 has been marked as a duplicate of this bug. ***

*** Bug 450981 has been marked as a duplicate of this bug. ***

*** Bug 450916 has been marked as a duplicate of this bug. ***

*** Bug 451540 has been marked as a duplicate of this bug. ***