Bug 357479 - crash if I close the splitscreen while focused on it and then change the view
Summary: crash if I close the splitscreen while focused on it and then change the view
Status: RESOLVED FIXED
Alias: None
Product: dolphin
Classification: Applications
Component: split view (show other bugs)
Version: 15.12.0
Platform: Arch Linux Linux
: NOR crash
Target Milestone: ---
Assignee: Dolphin Bug Assignee
URL:
Keywords:
: 358015 361383 361946 362500 362506 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-01-03 17:22 UTC by Thomas
Modified: 2016-05-23 07:09 UTC (History)
14 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Backtrace, no debug symbols (5.05 KB, text/plain)
2016-01-15 23:18 UTC, Elvis Angelaccio
Details
valgrind log (15.09 KB, text/plain)
2016-01-16 10:53 UTC, Elvis Angelaccio
Details
Possible fix (1.20 KB, patch)
2016-03-16 20:56 UTC, Emmanuel Pescosta
Details
stracktrace with debug symbols (6.12 KB, text/plain)
2016-04-08 23:28 UTC, Łukasz Żarnowiecki
Details
Weng's backtrace (duplicate #361946) (6.07 KB, text/plain)
2016-05-07 13:58 UTC, Elvis Angelaccio
Details
possible fix (861 bytes, patch)
2016-05-15 15:16 UTC, Martin Sandsmark
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas 2016-01-03 17:22:56 UTC
After I close the splitscreen, and the closed part was focused before closing, and changing the view, the application crashes. I think it tries to change the view of the closed side, because the bug does not appear if the closed part of the splitscreen wasn't focused.

Reproducible: Always

Steps to Reproduce:
1. Splitscreen on
2. Click on the new appeared side
3. Close splitscreen
4. Change the view mode

Actual Results:  
Dolphin crashes with a short delay

Expected Results:  
View is changed correctly, so it just shouldnt crash.
Comment 1 Elvis Angelaccio 2016-01-15 23:16:35 UTC
Same bug here, also on Archlinux starting from 15.12 release.

I also found another way to reproduce the crash (the backtrace looks the same):

1. Open the split view
2. Close the split view
3. Reload (F5) -> Crash
Comment 2 Elvis Angelaccio 2016-01-15 23:18:28 UTC
Created attachment 96668 [details]
Backtrace, no debug symbols

I'm attaching the backtrace I get, but I don't have debug symbols so it won't be that useful.
Comment 3 Elvis Angelaccio 2016-01-16 10:52:06 UTC
Weird, I cannot reproduce it on a openSUSE live system (tumbleweed, dolphin 15.12).
However, I can reproduce it every time here on archlinux.
Very annoying, because it feels like dolphin randomly crashes.
For instance, as soon as Firefox finished to download the above opensuse ISO, dolphin crashed with the very same backtrace.
Comment 4 Elvis Angelaccio 2016-01-16 10:53:13 UTC
Created attachment 96680 [details]
valgrind log

Valgrind log, I hope that will be more useful than my previous backtrace.
Comment 5 Emmanuel Pescosta 2016-01-16 10:58:03 UTC
(In reply to Elvis Angelaccio from comment #4)
> Created attachment 96680 [details]
> valgrind log
> 
> Valgrind log, I hope that will be more useful than my previous backtrace.

Thanks for the really helpful valgrind log :)
The problem is that KFileItemModelRolesUpdater doesn't kill all running preview jobs on destruction.
Comment 6 Emmanuel Pescosta 2016-01-16 11:02:04 UTC
(In reply to Emmanuel Pescosta from comment #5)
> (In reply to Elvis Angelaccio from comment #4)
> > Created attachment 96680 [details]
> > valgrind log
> > 
> > Valgrind log, I hope that will be more useful than my previous backtrace.
> 
> Thanks for the really helpful valgrind log :)
> The problem is that KFileItemModelRolesUpdater doesn't kill all running
> preview jobs on destruction.

Ok update -> correctly kill
Comment 7 Elvis Angelaccio 2016-02-04 08:54:26 UTC
Any news on this? I tried to have a look at the code, but without success. If you can point me in the right direction, I can try again and hopefully submit a patch...
Comment 8 Elvis Angelaccio 2016-02-11 13:32:29 UTC
*** Bug 358015 has been marked as a duplicate of this bug. ***
Comment 9 Emmanuel Pescosta 2016-03-16 20:56:52 UTC
Created attachment 97929 [details]
Possible fix
Comment 10 Emmanuel Pescosta 2016-03-16 20:57:20 UTC
(In reply to Elvis Angelaccio from comment #7)
> Any news on this? I tried to have a look at the code, but without success.
> If you can point me in the right direction, I can try again and hopefully
> submit a patch...

Sorry for the late reply.

Can you please try if the attached patch fixes the crash?
Comment 11 Emmanuel Pescosta 2016-03-16 21:00:35 UTC
(In reply to Emmanuel Pescosta from comment #10)
> (In reply to Elvis Angelaccio from comment #7)
> > Any news on this? I tried to have a look at the code, but without success.
> > If you can point me in the right direction, I can try again and hopefully
> > submit a patch...
> 
> Sorry for the late reply.
> 
> Can you please try if the attached patch fixes the crash?

Hmm the failed signal was the problem (I should re-read the back-trace before I try to fix it, my memory was wrong :/) 
Ok this patch will not help, I'll debug it this weekend.
Comment 12 Elvis Angelaccio 2016-03-16 21:05:01 UTC
(In reply to Emmanuel Pescosta from comment #11)
> (In reply to Emmanuel Pescosta from comment #10)
> > (In reply to Elvis Angelaccio from comment #7)
> > > Any news on this? I tried to have a look at the code, but without success.
> > > If you can point me in the right direction, I can try again and hopefully
> > > submit a patch...
> > 
> > Sorry for the late reply.
> > 
> > Can you please try if the attached patch fixes the crash?
> 
> Hmm the failed signal was the problem (I should re-read the back-trace
> before I try to fix it, my memory was wrong :/) 
> Ok this patch will not help, I'll debug it this weekend.

Yep, unfortunately does not fix the crash.
Comment 13 Christoph Feck 2016-04-06 00:27:03 UTC
*** Bug 361383 has been marked as a duplicate of this bug. ***
Comment 14 Łukasz Żarnowiecki 2016-04-08 23:28:36 UTC
Created attachment 98299 [details]
stracktrace with debug symbols

Still on 15.12.3

I will try to dig a little bit by myself.
Comment 15 Kay Simon 2016-04-27 08:07:52 UTC
Still on 16.04

KDE Frameworks 5.21.0
Qt 5.6.0
Comment 16 Elvis Angelaccio 2016-05-07 13:48:54 UTC
*** Bug 362506 has been marked as a duplicate of this bug. ***
Comment 17 Elvis Angelaccio 2016-05-07 13:49:59 UTC
*** Bug 362500 has been marked as a duplicate of this bug. ***
Comment 18 Elvis Angelaccio 2016-05-07 13:54:30 UTC
*** Bug 361946 has been marked as a duplicate of this bug. ***
Comment 19 Elvis Angelaccio 2016-05-07 13:58:24 UTC
Created attachment 98830 [details]
Weng's backtrace (duplicate #361946)

The Weng's backtrace in duplicate #361946 is more detailed, it looks like the crash happens when appending to the 'overlays' stringlist?
Comment 20 Elvis Angelaccio 2016-05-08 09:27:00 UTC
After some more digging, I found out that the bug is triggered by the Owncloud's overlay plugin [1], which is why not everyone can reproduce this crash. I uninstalled the owncloud-client and the bug was gone.

I'm adding Olivier in CC, who added the plugin in commit 800d5114 [2] and maybe can help us to understand whether the bug is in Dolphin or in Owncloud.


[1]: on my system: /usr/lib/qt/plugins/kf5/overlayicon/ownclouddolphinoverlayplugin.so
[2]: https://quickgit.kde.org/?p=dolphin.git&a=commit&h=800d5114cb1a9b7d5c874f382cc831b4f142469b
Comment 21 Antonio Rojas 2016-05-08 09:58:18 UTC
I'm getting the same crash with the owncloud plugin disabled.
Comment 22 Elvis Angelaccio 2016-05-08 12:52:27 UTC
(In reply to Antonio Rojas from comment #21)
> I'm getting the same crash with the owncloud plugin disabled.

What do you mean with disabled? Do you have owncloud-client installed?

(I'm not sure this plugin can be disabled in the Services page of the settings dialog)
Comment 23 Antonio Rojas 2016-05-08 13:38:40 UTC
(In reply to Elvis Angelaccio from comment #22)
> (In reply to Antonio Rojas from comment #21)
> > I'm getting the same crash with the owncloud plugin disabled.
> 
> What do you mean with disabled? Do you have owncloud-client installed?
> 
> (I'm not sure this plugin can be disabled in the Services page of the
> settings dialog)

Right, sorry. It seems the checkbox only disables the context menu, not the autial plugin.
Comment 24 Martin Sandsmark 2016-05-15 14:44:58 UTC
the problem is that the owncloud plugin is destroyed (twice?) when the split is closed.

            QString pluginName = QString::fromLocal8Bit(plugin->metaObject()->className()) + " " + plugin->objectName();
            connect(plugin, &QObject::destroyed, [=](QObject *obj) {
                qWarning() << pluginName << obj << "destroyed";
            });

gives:

"OwncloudDolphinPlugin " QObject(0x190a040) destroyed
"OwncloudDolphinPlugin " QObject(0x190a040) destroyed
Comment 25 Martin Sandsmark 2016-05-15 14:58:45 UTC
... and the reason is that the KFileItemModelRolesUpdater for both splits get the same OwncloudDolphinPlugin object from «KPluginLoader::instantiatePlugins(QStringLiteral("kf5/overlayicon"), nullptr, this);» the first two times it is called:

KFileItemModelRolesUpdater(0x1bde1a0) Creating "OwncloudDolphinPlugin " OwncloudDolphinPlugin(0x1cb9420)

(open split)

KFileItemModelRolesUpdater(0x20e7f90) Creating "OwncloudDolphinPlugin " OwncloudDolphinPlugin(0x1cb9420)
Comment 26 Martin Sandsmark 2016-05-15 15:16:27 UTC
Created attachment 98990 [details]
possible fix

This should fix it.

The problem is that instantiatePlugin() only returns a new object if the old one is deleted, so we shouldn't expect it to create a new one for every call.
Comment 27 Elvis Angelaccio 2016-05-15 16:02:33 UTC
(In reply to Martin Sandsmark from comment #26)
> Created attachment 98990 [details]
> possible fix
> 
> This should fix it.
> 
> The problem is that instantiatePlugin() only returns a new object if the old
> one is deleted, so we shouldn't expect it to create a new one for every call.

Works for me. Thanks!
Comment 28 Martin Sandsmark 2016-05-15 16:16:31 UTC
Up for review: https://git.reviewboard.kde.org/r/127930/
Comment 29 Łukasz Żarnowiecki 2016-05-15 17:07:00 UTC
Works for me to, thanks.
Comment 30 Emmanuel Pescosta 2016-05-17 10:46:57 UTC
(In reply to Martin Sandsmark from comment #24)
> the problem is that the owncloud plugin is destroyed (twice?) when the split
> is closed.
> 
>             QString pluginName =
> QString::fromLocal8Bit(plugin->metaObject()->className()) + " " +
> plugin->objectName();
>             connect(plugin, &QObject::destroyed, [=](QObject *obj) {
>                 qWarning() << pluginName << obj << "destroyed";
>             });
> 
> gives:
> 
> "OwncloudDolphinPlugin " QObject(0x190a040) destroyed
> "OwncloudDolphinPlugin " QObject(0x190a040) destroyed

Thanks for debugging the problem! :)

This is a different bug, because I don't have anything Owcloud-related installed on my system, but I can also trigger a crash when closing the split view sometimes.
Comment 31 Martin Sandsmark 2016-05-21 16:12:03 UTC
Git commit b1471bbd09d88da3ffe8159075b3108bf9586220 by Martin T. H. Sandsmark.
Committed on 21/05/2016 at 16:11.
Pushed by sandsmark into branch 'master'.

Fix crash when closing split view with ownCloud plugin loaded

KPluginLoader::instantiatePlugins() wraps QPluginLoader::instace(),
which doesn't return a new object for each call, so if we set the
KFileItemModelRolesUpdater instance as parent to the plugin the shared
instance will be deleted leading to crashes when other instances of
KFileItemModelRolesUpdater tries to use their plugin objects.

To fix this, set the QApplication as a parent.

REVIEW: 127930

M  +1    -1    src/kitemviews/kfileitemmodelrolesupdater.cpp

http://commits.kde.org/dolphin/b1471bbd09d88da3ffe8159075b3108bf9586220
Comment 32 Martin Sandsmark 2016-05-21 16:14:42 UTC
(In reply to Emmanuel Pescosta from comment #30)
> This is a different bug, because I don't have anything Owcloud-related
> installed on my system, but I can also trigger a crash when closing the
> split view sometimes.

Do you have any other plugins installed?
Comment 33 Martin Sandsmark 2016-05-21 16:16:46 UTC
Git commit bed16191b5e9253d8658c0dac0d336b3dab5e0e3 by Martin T. H. Sandsmark.
Committed on 21/05/2016 at 16:16.
Pushed by sandsmark into branch 'Applications/16.04'.

Fix crash when closing split view with ownCloud plugin loaded

KPluginLoader::instantiatePlugins() wraps QPluginLoader::instace(),
which doesn't return a new object for each call, so if we set the
KFileItemModelRolesUpdater instance as parent to the plugin the shared
instance will be deleted leading to crashes when other instances of
KFileItemModelRolesUpdater tries to use their plugin objects.

To fix this, set the QApplication as a parent.

REVIEW: 127930

M  +1    -1    src/kitemviews/kfileitemmodelrolesupdater.cpp

http://commits.kde.org/dolphin/bed16191b5e9253d8658c0dac0d336b3dab5e0e3
Comment 34 Emmanuel Pescosta 2016-05-21 17:08:36 UTC
(In reply to Martin Sandsmark from comment #32)
> Do you have any other plugins installed?

No I don't have any plugins installed that are KOverlayIconPlugin based.

Also the backtraces above don't mention anything related to KOverlayIconPlugin. I think that killing the preview job triggers the crash, because the job emits a signal although kill should not emit anything.
Comment 35 Martin Sandsmark 2016-05-23 07:09:18 UTC
> Also the backtraces above don't mention anything related to KOverlayIconPlugin. I think that killing the preview job triggers the crash, because the job emits a signal although kill should not emit anything.

All the backtraces I can see point at «overlays.append(it->getOverlays(item.url()));» in src/kitemviews/kfileitemmodelrolesupdater.cpp. Which backtraces are you referring to?