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

List:       kde-commits
Subject:    branches/kdepim/enterprise/kdepim/kmail
From:       Sergio Luis Martins <iamsergio () gmail ! com>
Date:       2010-10-07 15:36:10
Message-ID: 20101007153611.007AEAC892 () svn ! kde ! org
[Download RAW message or body]

SVN commit 1183491 by smartins:

Fix crash in removeMsg().
Part of kolab/issue4571.

In FolderStorage::removeMsg(const QPtrList<KMMsgBase>& msgList, bool imapQuiet)
we are iterating over a list of message pointers, and removing messages one by
one.

These pointers are exactly the same as the ones in mMsgList ( in KMFolderIndex
), i.e. they have the same value, point to the same address.

While iterating in FolderStorage::removeMsg(), and while removing the first
message, there's an: emit msgRemoved( idx, msgIdMD5 );

KMHeaders catches this signal and tries to highlight the next message, when it
does that, a call to KMFolderMailDir::getMsg() is made, which changes our
mMsgList pointers: mMsgList.set(idx,&msg->toMsgBase()); and index zero has now
another address.

Back to our initial iteration, processing the second message, find() fails, it
can't find the message pointer inside mMsgList, because KMHeaders changed it
when the first element was removed.

 M  +10 -0     folderstorage.cpp  
 M  +15 -1     folderstorage.h  
 M  +22 -0     kmheaders.cpp  
 M  +7 -0      kmheaders.h  


--- branches/kdepim/enterprise/kdepim/kmail/folderstorage.cpp #1183490:1183491
@@ -357,8 +357,17 @@
 //-----------------------------------------------------------------------------
 void FolderStorage::removeMsg(const QPtrList<KMMsgBase>& msgList, bool imapQuiet)
 {
+
+  emit batchRemovingStarted();
+  int i = 0;
   for( QPtrListIterator<KMMsgBase> it( msgList ); *it; ++it )
   {
+    if ( i == msgList.count() - 1 ) {
+      // For a batch of messages, we only want the last one
+      // to be processed by KMHeaders
+      emit batchRemovingFinished();
+    }
+
     int idx = find(it.current());
 
     if ( idx == -1 ) {
@@ -370,6 +379,7 @@
     }
 
     removeMsg(idx, imapQuiet);
+    ++i;
   }
 }
 
--- branches/kdepim/enterprise/kdepim/kmail/folderstorage.h #1183490:1183491
@@ -502,11 +502,25 @@
   void folderSizeChanged();
 
   /**
-   *  Emiitted when the sync state, i.e. mailCheckInProgress(), changes.
+   *  Emitted when the sync state, i.e. mailCheckInProgress(), changes.
    *  Currently only supported for disconnected IMAP.
    */
   void syncStateChanged();
 
+  /**
+     Emitted when we are removing a list of messages. This is caught
+     for example by KMHeaders so it doesn't change any pointer in mMsgList.
+     Pointers to messages that we are removing must correspond to the ones in mMsgList.
+     More information in: https://issues.kolab.org/msg26709
+  */
+  void batchRemovingStarted();
+
+  /**
+     Emitted when we finish removing a list of messages.
+     It's now safe to call getMsg() and change contents of mMsgList.
+  */
+  void batchRemovingFinished();
+
 public slots:
   /** Incrementally update the index if possible else call writeIndex */
   virtual int updateIndex() = 0;
--- branches/kdepim/enterprise/kdepim/kmail/kmheaders.cpp #1183490:1183491
@@ -15,6 +15,7 @@
 #include "kmmsgdict.h"
 #include "kmdebug.h"
 #include "kmfoldertree.h"
+#include "folderstorage.h"
 #include "folderjob.h"
 using KMail::FolderJob;
 #include "actionscheduler.h"
@@ -120,6 +121,7 @@
   mSortDescending = false;
   mSortInfo.ascending = false;
   mReaderWindowActive = false;
+  mBatchRemovingInProgress = false;
   mRoot = new SortCacheItem;
   mRoot->setId(-666); //mark of the root!
   setStyleDependantFrameWidth();
@@ -710,6 +712,10 @@
                  this, SLOT(msgAdded(int)));
       disconnect(mFolder, SIGNAL( msgRemoved( int, QString ) ),
                  this, SLOT( msgRemoved( int, QString ) ) );
+      disconnect(mFolder->storage(), SIGNAL( batchRemovingStarted() ),
+                 this, SLOT( batchRemovingStarted() ) );
+      disconnect(mFolder->storage(), SIGNAL( batchRemovingFinished( ) ),
+                 this, SLOT( batchRemovingFinished() ) );
       disconnect(mFolder, SIGNAL(changed()),
                  this, SLOT(msgChanged()));
       disconnect(mFolder, SIGNAL(cleared()),
@@ -743,6 +749,10 @@
               this, SLOT(msgAdded(int)));
       connect(mFolder, SIGNAL(msgRemoved(int,QString)),
               this, SLOT(msgRemoved(int,QString)));
+      connect(mFolder->storage(), SIGNAL( batchRemovingStarted() ),
+              this, SLOT( batchRemovingStarted() ) );
+      connect(mFolder->storage(), SIGNAL( batchRemovingFinished( ) ),
+              this, SLOT( batchRemovingFinished() ) );
       connect(mFolder, SIGNAL(changed()),
               this, SLOT(msgChanged()));
       connect(mFolder, SIGNAL(cleared()),
@@ -1573,6 +1583,8 @@
     setSelected( item, true );
     setSelectionAnchor( currentItem() );
     mPrevCurrent = 0;
+    // read folderstorage.h:batchRemovingStarts() for more info on mBatchRemovingInProgress
+    if ( !mBatchRemovingInProgress )
     highlightMessage( item, false);
   }
 
@@ -3650,4 +3662,14 @@
   return list;
 }
 
+void KMHeaders::batchRemovingStarted()
+{
+  mBatchRemovingInProgress = true;
+}
+
+void KMHeaders::batchRemovingFinished()
+{
+  mBatchRemovingInProgress = false;
+}
+
 #include "kmheaders.moc"
--- branches/kdepim/enterprise/kdepim/kmail/kmheaders.h #1183490:1183491
@@ -374,6 +374,9 @@
 
   void updateActions();
 
+  void batchRemovingStarted();
+  void batchRemovingFinished();
+
 private:
   /** Is equivalent to clearing the list and inserting an item for
       each message in the current folder */
@@ -471,5 +474,9 @@
   // copied messages
   QValueList<Q_UINT32> mCopiedMessages;
   bool mMoveMessages;
+
+  // Info about this in folderstorage.h:batchRemovingStarts()
+  bool mBatchRemovingInProgress;
+
 }; // class
 #endif
[prev in list] [next in list] [prev in thread] [next in thread]