[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kmail-devel
Subject:    Re: [BT] KMail branch Crash
From:       Ingo =?iso-8859-15?q?Kl=F6cker?= <kloecker () kde ! org>
Date:       2006-06-30 7:36:13
Message-ID: 200606300936.44838 () helena ! mathA ! rwth-aachen ! de
[Download RAW message or body]

[Attachment #2 (multipart/signed)]

[Attachment #4 (multipart/mixed)]


Hi Allen,

am Freitag, 30. Juni 2006 01:09 schrieb Allen Winter:
> Howdy KMail Gurus,
>
> Attached is a backtrace from a KMail crash that just happened to me.
> I'm using a pretty recent version from branch... maybe a week old.
>
> Not sure what to make of it.

Was it a segmentation fault or an abort? In case of a segmentation fault 
my only guess is that the progress item might have been destroyed too 
early.

Related bug reports:
http://bugs.kde.org/show_bug.cgi?id=110487
http://bugs.kde.org/show_bug.cgi?id=118112
http://bugs.kde.org/show_bug.cgi?id=119112
http://bugs.kde.org/show_bug.cgi?id=121384
http://bugs.kde.org/show_bug.cgi?id=127210

I think I know what might happen:
A check for new mail starts. The check takes some time. The next check 
is started, but since we are already checking 
PopAccount::processNewMail() calls
  KMAccount::checkDone( false, CheckIgnored ) which calls
    mMailCheckProgressItem->setComplete() and then sets
    mMailCheckProgressItem = 0.
Now I guess that setComplete() doesn't return fast enough so that
PopAccount::slotJobFinished() is invoked because the 1st check just 
finished and this checks for mMailCheckProgressItem == 0. If it's not 0 
then it uses the progress item and this eventually leads to the crash.

Hmm, doesn't seem to be right. As soon as a check is started the timer 
is stopped. So another check can't start (at least not a timer invoked 
one and the bug reports clearly indicate that the users didn't do a 
manual check). Maybe slotFinished() is invoked twice for some reason?

Attached is the patch I'm going to commit when I'm home again.

Regards,
Ingo

["fix-popaccount-crashes.diff" (text/x-diff)]

Index: kmaccount.cpp
===================================================================
--- kmaccount.cpp	(Revision 556419)
+++ kmaccount.cpp	(Arbeitskopie)
@@ -444,8 +444,11 @@ void KMAccount::checkDone( bool newmail,
   if (mTimer)
     mTimer->start(mInterval*60000);
   if ( mMailCheckProgressItem ) {
-    mMailCheckProgressItem->setComplete(); // that will delete it
+    // set mMailCheckProgressItem = 0 before calling setComplete() to prevent
+    // a race condition
+    ProgressItem *savedMailCheckProgressItem = mMailCheckProgressItem;
     mMailCheckProgressItem = 0;
+    savedMailCheckProgressItem->setComplete(); // that will delete it
   }
 
   emit newMailsProcessed( mNewInFolder );
Index: popaccount.cpp
===================================================================
--- popaccount.cpp	(Revision 556419)
+++ popaccount.cpp	(Arbeitskopie)
@@ -348,8 +348,9 @@ void PopAccount::slotProcessPendingMsgs(
 void PopAccount::slotAbortRequested()
 {
   if (stage == Idle) return;
-  disconnect( mMailCheckProgressItem, SIGNAL( progressItemCanceled( \
                KPIM::ProgressItem* ) ),
-           this, SLOT( slotAbortRequested() ) );
+  if ( mMailCheckProgressItem )
+    disconnect( mMailCheckProgressItem, SIGNAL( progressItemCanceled( \
KPIM::ProgressItem* ) ), +                this, SLOT( slotAbortRequested() ) );
   stage = Quit;
   if (job) job->kill();
   job = 0;
@@ -655,7 +656,8 @@ void PopAccount::slotJobFinished() {
     processMsgsTimer.start(processingDelay);
   }
   else if (stage == Retr) {
-    mMailCheckProgressItem->setProgress( 100 );
+    if ( mMailCheckProgressItem )
+      mMailCheckProgressItem->setProgress( 100 );
     processRemainingQueuedMessages();
 
     mHeaderDeleteUids.clear();
@@ -732,20 +734,22 @@ void PopAccount::slotJobFinished() {
     // If there are messages to delete then delete them
     if ( !idsOfMsgsToDelete.isEmpty() ) {
       stage = Dele;
-      mMailCheckProgressItem->setStatus(
-        i18n( "Fetched 1 message from %1. Deleting messages from server...",
-              "Fetched %n messages from %1. Deleting messages from server...",
-              numMsgs )
-        .arg( mHost ) );
+      if ( mMailCheckProgressItem )
+        mMailCheckProgressItem->setStatus(
+          i18n( "Fetched 1 message from %1. Deleting messages from server...",
+                "Fetched %n messages from %1. Deleting messages from server...",
+                numMsgs )
+          .arg( mHost ) );
       url.setPath("/remove/" + idsOfMsgsToDelete.join(","));
       kdDebug(5006) << "url: " << url.prettyURL() << endl;
     } else {
       stage = Quit;
-      mMailCheckProgressItem->setStatus(
-        i18n( "Fetched 1 message from %1. Terminating transmission...",
-              "Fetched %n messages from %1. Terminating transmission...",
-              numMsgs )
-        .arg( mHost ) );
+      if ( mMailCheckProgressItem )
+        mMailCheckProgressItem->setStatus(
+          i18n( "Fetched 1 message from %1. Terminating transmission...",
+                "Fetched %n messages from %1. Terminating transmission...",
+                numMsgs )
+          .arg( mHost ) );
       url.setPath(QString("/commit"));
       kdDebug(5006) << "url: " << url.prettyURL() << endl;
     }
@@ -760,11 +764,12 @@ void PopAccount::slotJobFinished() {
       mUidsOfNextSeenMsgsDict.remove( mUidForIdMap[*it] );
     }
     idsOfMsgsToDelete.clear();
-    mMailCheckProgressItem->setStatus(
-      i18n( "Fetched 1 message from %1. Terminating transmission...",
-            "Fetched %n messages from %1. Terminating transmission...",
-            numMsgs )
-      .arg( mHost ) );
+    if ( mMailCheckProgressItem )
+      mMailCheckProgressItem->setStatus(
+        i18n( "Fetched 1 message from %1. Terminating transmission...",
+              "Fetched %n messages from %1. Terminating transmission...",
+              numMsgs )
+        .arg( mHost ) );
     KURL url = getUrl();
     url.setPath(QString("/commit"));
     job = KIO::get( url, false, false );
@@ -783,8 +788,11 @@ void PopAccount::slotJobFinished() {
       int numMessages = canceled ? indexOfCurrentMsg : idsOfMsgs.count();
       BroadcastStatus::instance()->setStatusMsgTransmissionCompleted(
         this->name(), numMessages, numBytes, numBytesRead, numBytesToRead, \
                mLeaveOnServer, mMailCheckProgressItem );
-      mMailCheckProgressItem->setComplete();
+      // set mMailCheckProgressItem = 0 before calling setComplete() to prevent
+      // a race condition
+      ProgressItem *savedMailCheckProgressItem = mMailCheckProgressItem;
       mMailCheckProgressItem = 0;
+      savedMailCheckProgressItem->setComplete(); // that will delete it
       checkDone( ( numMessages > 0 ), canceled ? CheckAborted : CheckOK );
     }
   }
@@ -873,7 +881,9 @@ void PopAccount::slotData( KIO::Job* job
       numMsgBytesRead = curMsgLen;
     numBytesRead += numMsgBytesRead - oldNumMsgBytesRead;
     dataCounter++;
-    if (dataCounter % 5 == 0)
+    if ( mMailCheckProgressItem &&
+         ( dataCounter % 5 == 0 ||
+           ( indexOfCurrentMsg + 1 == numMsgs && numMsgBytesRead == curMsgLen ) ) )
     {
       QString msg;
       if (numBytes != numBytesToRead && mLeaveOnServer)


[Attachment #8 (application/pgp-signature)]

_______________________________________________
KMail developers mailing list
KMail-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmail-devel


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic