Bug 57991 - crash in KDirListerCache::listDir if two KDirListers work on the same directory
Summary: crash in KDirListerCache::listDir if two KDirListers work on the same directory
Status: RESOLVED FIXED
Alias: None
Product: kio
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR crash
Target Milestone: ---
Assignee: Michael Brade
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-05-02 00:31 UTC by Aurelien Gateau
Modified: 2003-05-08 05:14 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
Excerpt of a trace when everything goes well. (1.38 KB, text/plain)
2003-05-02 00:32 UTC, Aurelien Gateau
Details
Excerpt of a trace when it crashes. (1.31 KB, text/plain)
2003-05-02 00:33 UTC, Aurelien Gateau
Details
Proposed patch (773 bytes, patch)
2003-05-02 00:33 UTC, Aurelien Gateau
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Aurelien Gateau 2003-05-02 00:31:23 UTC
Version:            (using KDE KDE 3.1.1)
Installed from:    Compiled From Sources
Compiler:          gcc 3.2 
OS:          Linux

This bug is not 100% reproducible, but it happens quite often when I work on Gwenview. Here is how I best reproduce it:
- Create a folder with only one image in it.
- Run "gwenview /path/to/the/folder" several times.
Running Gwenview 10 times usually result in 3 to 5 crashes.

Using a debug version of KDE libs, I found it crashes in KDirListerCache::listDir() on this assert:

      // _url is already in use, so there is already an entry in urlsCurrentlyHeld
      assert( urlsCurrentlyHeld[_url.url()] );
      urlsCurrentlyHeld[_url.url()]->append( lister );

In Gwenview I run two KDirListers, one is run by KFileTreeView, the other by my own file view. The crash seems to happen if one of the KDirLister ends too soon (see the two attached trace excerpts).

After some more investigation, I found out that KDirListerCache::slotResult() might be the culprit. Currently it works like this:
1. Remove the KDirLister from urlsCurrentlyListed
2. Emit the completed or canceled signals
3. Append the KDirLister to urlsCurrentlyHeld

So when the signals are emitted in step 2, the KDirLister is neither in urlsCurrentlyListed, nor in urlsCurrentlyHeld. The attached patch simply exchange step 2 and step 3. This seems to fix the bug, but since I do not know this part of the source code very well, it might break other stuff.
Comment 1 Aurelien Gateau 2003-05-02 00:32:35 UTC
Created attachment 1470 [details]
Excerpt of a trace when everything goes well.
Comment 2 Aurelien Gateau 2003-05-02 00:33:03 UTC
Created attachment 1471 [details]
Excerpt of a trace when it crashes.
Comment 3 Aurelien Gateau 2003-05-02 00:33:39 UTC
Created attachment 1472 [details]
Proposed patch
Comment 4 Maksim Orlovich 2003-05-02 01:40:05 UTC
Aurelien: One semi-related question... I believe that some Konqueror crash reports 
are related to having two KDirListers working on the same directory.... Does this 
particular crash when happening w/o debug output produce a crash within 
KURL::url or similar KURL method? 
 
At any rate, I think it'd be better you send this to core-devel or kfm-devel, to make 
sure dfaure doesn't miss this.. 
 
 
Comment 5 Maksim Orlovich 2003-05-02 01:41:03 UTC
Doh, I should have checked my inbox for the reassign, sorry.  
<feels stupid> 
 
Comment 6 Michael Brade 2003-05-06 16:36:47 UTC
Sick KDirLister usage ;-) Yeah, apart from that, very well spotted, thanks! And thanks a lot for 
the detailed bugreport. Please apply the patch, I can't think of any bad side effects right now. 
 
And if you want, close this report, maybe just with a CCMAIL :) 
 
Regards, 
  Michael 
Comment 7 Aurelien Gateau 2003-05-06 22:11:39 UTC
I would be happy to apply the patch, but I don't run KDE CVS (this patch was 
against KDE 3.1.1), so I would like to avoid committing some code without compiling 
it. Can you do it for me? 
 
Aur
Comment 8 Michael Brade 2003-05-08 05:14:04 UTC
Subject: kdelibs/kio/kio

CVS commit by brade: 

His wish is my command, committing on behalf of Aurelien: fix #57991,
crash in KDirListerCache::listDir when called just after another 
KDirLister's job finished and KDirListerCache::slotResult is emitting
the signals.

CCMAIL: 57991-done@bugs.kde.org


  M +7 -6      kdirlister.cpp   1.162


--- kdelibs/kio/kio/kdirlister.cpp  #1.161:1.162
@@ -882,9 +882,8 @@ void KDirListerCache::slotEntries( KIO::
     else if ( name != dotdot )
     {
-      //kdDebug(7004)<< "Adding " << url.prettyURL() << endl;
-
       KFileItem* item = new KFileItem( *it, url, delayedMimeTypes, true );
       Q_ASSERT( item );
 
+      //kdDebug(7004)<< "Adding item: " << item->url() << endl;
       dir->lstItems->append( item );
 
@@ -916,4 +915,10 @@ void KDirListerCache::slotResult( KIO::J
   Q_ASSERT( listers );
 
+  // move the directory to the held directories, do it before emitting
+  // the signals to make sure it exists in KDirListerCache in case someone 
+  // calls listDir during the signal emission
+  Q_ASSERT( !urlsCurrentlyHeld[jobUrlStr] );
+  urlsCurrentlyHeld.insert( jobUrlStr, listers );
+
   KDirLister *kdl;
 
@@ -947,8 +952,4 @@ void KDirListerCache::slotResult( KIO::J
     }
   }
-
-  // move the directory to the held directories
-  Q_ASSERT( !urlsCurrentlyHeld[jobUrlStr] );
-  urlsCurrentlyHeld.insert( jobUrlStr, listers );
 
   // TODO: hmm, if there was an error and job is a parent of one or more