[frameworks-kio] [Bug 433347] Dolphin may crash when repeatedly opening and closing dialog

[Harald Sitter]

https://bugs.kde.org/show_bug.cgi?id=433347

Harald Sitter <sitter at kde.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|general                     |general
             Status|REPORTED                    |CONFIRMED
            Product|dolphin                     |frameworks-kio
                 CC|                            |kdelibs-bugs at kde.org,
                   |                            |sitter at kde.org
            Version|20.12.2                     |git master
           Assignee|dolphin-bugs-null at kde.org   |kio-bugs-null at kde.org
     Ever confirmed|0                           |1

--- Comment #7 from Harald Sitter <sitter at kde.org> ---
> 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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.