Bug 159343 - Device notifer fails an assertion and makes plasma crash
Summary: Device notifer fails an assertion and makes plasma crash
Status: RESOLVED FIXED
Alias: None
Product: plasma4
Classification: Plasma
Component: general (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR crash
Target Milestone: ---
Assignee: Marco Martin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-03-15 11:45 UTC by John Varouhakis
Modified: 2008-03-16 18:21 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
backtrace of the crash (5.58 KB, text/plain)
2008-03-15 11:46 UTC, John Varouhakis
Details
Crash log (6.11 KB, text/plain)
2008-03-15 21:14 UTC, Bedrich Lakomy
Details
A small patch for devicenotifer (411 bytes, patch)
2008-03-16 06:34 UTC, Christopher Blauvelt
Details
another micro-patch (659 bytes, patch)
2008-03-16 12:05 UTC, Marco Martin
Details
Crash output (7.07 KB, text/plain)
2008-03-16 15:29 UTC, Vincenzo Di Massa
Details
Disconnects a source before removing the row (731 bytes, patch)
2008-03-16 18:02 UTC, John Varouhakis
Details

Note You need to log in before you can comment on or make changes to this bug.
Description John Varouhakis 2008-03-15 11:45:24 UTC
Version:            (using Devel)
Installed from:    Compiled sources
Compiler:          gcc (GCC) 4.2.3 (Debian 4.2.3-2) 
OS:                Linux

After yesterdays updates, I can't use device-notifier since it
keeps crashing taking plasma down with it.

I'm getting these messages in konsole:

plasma(27432) DeviceNotifier::indexForUdi: We should not be here!
ASSERT: "index.isValid()" in file /media/hdb1/factory/kdesvn/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.cpp, line 296
Plasma crashed, attempting to automatically recover
KCrash: crashing... crashRecursionCounter = 2
KCrash: Application Name = plasma path = <unknown> pid = 27432
sock_file=/home/iovar/.kde4/socket-debian/kdeinit4__0
plasma(27431): Communication problem with  "plasma" , it probably crashed.


This happens on a Debian testing/unstable with KDE4 compiled from svn-trunk, using qt-copy.


Thanks.
Comment 1 John Varouhakis 2008-03-15 11:46:16 UTC
Created attachment 23901 [details]
backtrace of the crash
Comment 2 Bedrich Lakomy 2008-03-15 21:12:07 UTC
This (or something very similar) is happening to me too, unfortunately.

Fedora 8
gcc (GCC) 4.1.2 20070925 (Red Hat 4.1.2-33)
version: devel, svn revision approx 786000

All sources downloaded on 2008-03-15 from svn.
KDE4 compiled from svn-trunk using qt-copy. (I just followed the tutorial)
Comment 3 Bedrich Lakomy 2008-03-15 21:14:56 UTC
Created attachment 23912 [details]
Crash log

Crash log of my case
Comment 4 Christopher Blauvelt 2008-03-16 06:34:57 UTC
Created attachment 23916 [details]
A small patch for devicenotifer

Please try applying this patch to see if the problem is fixed.	I believe the
problem occurs when the device is removed but is somehow updated in the
soliddevice engine.
Comment 5 John Varouhakis 2008-03-16 09:19:04 UTC
I've applied the patch but I'm afraid that nothing has changed.
Still the same crash.

BTW, the problem doesn't happen when interacting with devices.
Simply loading device-notifier is enough to trigger it
(it never gets to the loading really, the moment I release
the mouse cursor plasma freezes and eventually crashes).

Comment 6 Bedrich Lakomy 2008-03-16 10:45:23 UTC
I've updated kdebase to rev 786180 and applied the patch. No change.

For me, the plasma crashes almost immediately KDE4 starts. I get to see the taskbar without icons, then it disappears and KDE Crash handler pops up with "plasma crashed with signal 6(SIGABRT)".
Comment 7 John Varouhakis 2008-03-16 10:59:45 UTC
>> For me, the plasma crashes almost immediately KDE4 starts

That's what I got at first, too, but then I just removed it 
manually from plasma-appletsrc, to get a working desktop.
Comment 8 Marco Martin 2008-03-16 12:04:05 UTC
can you try this second patch?
Comment 9 Marco Martin 2008-03-16 12:05:16 UTC
Created attachment 23917 [details]
another micro-patch

in this way shouldn't arrive data from dataengine before having inserted the
item in the model
Comment 10 Bedrich Lakomy 2008-03-16 12:42:46 UTC
Applied the second patch, still nothing :(

>> removed it manually from plasma-appletsrc
I cannot find the plasma-appletsrc file ("find / plasma-appletsrc -depth" returns no such file or directory). Where/how can i get this file?
Comment 11 John Varouhakis 2008-03-16 12:48:17 UTC
I'm afraid the second patch doesn't change anything either.
But it certainly has to do something with the soliddevice engine,
since if I remove the call to connect, the applet no longer crashes,
when I load it:

Index: devicenotifier.cpp
===================================================================
--- devicenotifier.cpp  (revision 785878)
+++ devicenotifier.cpp  (working copy)
@@ -330,7 +330,6 @@
     m_hotplugModel->insertRow(0, item);
     m_solidEngine->connectSource(name, this);

-    m_solidDeviceEngine->connectSource(name, this);

     //sets the "action" column
     QStandardItem *actionItem = new QStandardItem();
Comment 12 John Varouhakis 2008-03-16 12:51:47 UTC
Bedrich, it's in $KDEHOME/share/config/plasma-appletsrc.


Comment 13 Marco Martin 2008-03-16 13:04:29 UTC
does commenting out the else block around line 296 of devicenotifier.cpp resolves the problem?
unfortunately i'm going a little bit blindly on this because i can't reproduce the problem
i think the reference to a not existent item could happen either here or in osSourceRemoved, a variant of the first patch by Christopher could be disconnecting the source before removing the row, so something like this:
Index: devicenotifier.cpp
===================================================================
--- devicenotifier.cpp	(revision 785878)
+++ devicenotifier.cpp	(working copy)
@@ -343,13 +343,15 @@
 
 void DeviceNotifier::onSourceRemoved(const QString &name)
 {
+    m_solidEngine->disconnectSource(name, this);
+    m_solidDeviceEngine->disconnectSource(name, this);
+
     QModelIndex index = indexForUdi(name);
     Q_ASSERT(index.isValid());
     m_hotplugModel->removeRow(index.row());
     if (m_icon && m_hotplugModel->rowCount() == 0) {
         m_widget->hide();
     }
-
 }
 
 QModelIndex DeviceNotifier::indexForUdi(const QString &udi) const
Comment 14 Vincenzo Di Massa 2008-03-16 15:29:38 UTC
Created attachment 23919 [details]
Crash output

Before triggering the assert devicenotifier shows some 
"ERROR: sysntax error" lines
Can that be related?
Comment 15 Marco Martin 2008-03-16 16:04:19 UTC
SVN commit 786261 by mart:

after a device removal disconnect sources before removing items and some debug output when soliddevice engine is updated

CCBUG: 159343

 M  +4 -1      devicenotifier.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=786261
Comment 16 Marco Martin 2008-03-16 16:06:55 UTC
vincenzo: the syntax error is because of the way the siliddevice engine works, i think the check if the query is a device identifier and the check if it is a predicate should be swapped, i'll try that
Comment 17 John Varouhakis 2008-03-16 17:53:51 UTC
Ok, I think I found the cause :).

I have 6 partitions mounted, where the default limit 
is 4. So, something simple, like changing the default 
to a high value, in devicenotifier.cpp:83, will allow
the applet to load (it does in me).

I think the problem is in devicenotifier.cpp:325, where
the rows are removed without disconnecting the relevant sources,
so when the update happens, the name for the source isn't found,
resulting in the assertion.



Comment 18 John Varouhakis 2008-03-16 18:02:07 UTC
Created attachment 23920 [details]
Disconnects a source before removing the row
Comment 19 Marco Martin 2008-03-16 18:05:44 UTC
John: great job!
do you have an account? can you commit or otherwise i will commit it (with proper credits eh :))
Comment 20 Marco Martin 2008-03-16 18:05:52 UTC
John: great job!
do you have an account? can you commit or otherwise i will commit it (with proper credits eh :))
Comment 21 John Varouhakis 2008-03-16 18:14:08 UTC
No , I don't have an account, so go on and commit it .

Thanks for the help and have a great 
what's-left-from-your-weekend :) .
Comment 22 Marco Martin 2008-03-16 18:21:53 UTC
SVN commit 786308 by mart:

disconnect sources before removing a row (when the row limit is exceeded)
now it no longer causes a failed assertion. patch by John Varouhakis

BUG: 159343

 M  +6 -0      devicenotifier.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=786308