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

List:       kmail-devel
Subject:    Re: Reason for crashes when filtering mails
From:       Andreas Gungl <Andreas.Gungl () osp-dd ! de>
Date:       2007-02-26 8:45:49
Message-ID: 200702260945.49275 () osp-dd ! de
[Download RAW message or body]

Am Sunday 25 February 2007 schrieb Ingo Klöcker:
> On Sunday 25 February 2007 13:39, Andreas Gungl wrote:
> > This makes me wonder if idx == 0 is a valid index and how the assert
> > is meant to work. Of course, the real problem is mStorage being not
> > defined, but that's a bit beyond my knowledge of the folder stuff in
> > KMail.
> 
> 0 is definitely a valid index, so KMFolder::find() should return -1 in
> case of mStorage == NULL.

Fine. I was fooled by the current code in the 3.5 branch. I've seen Allen fixed 
that in kpim+, but such changes should be applied to the 3.5 branch immediately.

> msgBase->parent()->find( msgBase ) >= 0 always has to be true because
> obviously msgBase must be in its parent (i.e. its folder) and thus it
> must be found in the folder. mStorage being NULL is of course a serious
> problem and I have no idea how that could ever happen because mStorage
> is set in the c'tor of KMFolder and it's never set to 0 anywhere.
> 
> I'm sorry, but I don't see what could possibly go wrong. To prevent the
> crash I suggest adding a check for idx != -1 after the assert and
> disabling the assert in the release build.
> 
> It would be great if you could debug this a bit more in order to find
> out why mStorage is NULL.

The problem is in the access of msgBase->parent(). I've slightly modified the
code from the 3.5 branch. I've added the changes made by Allen plus some
additional code to get an idea what's going wrong. See the attached diff.

Here's what valgrind gave me when triggering the crash:

==11728==
==11728== Invalid read of size 4
==11728==    at 0x42404DA: KMMsgBase::parent() const (kmmsgbase.h:146)
==11728==    by 0x42C1295: KMHeaders::applyFiltersOnMsg() (kmheaders.cpp:1378)
==11728==    by 0x446C609: KMMainWidget::slotApplyFilters() (kmmainwidget.cpp:1668)
==11728==    by 0x4484B61: KMMainWidget::qt_invoke(int, QUObject*) \
(kmmainwidget.moc:575) ==11728==    by 0x5EF73CC: \
QObject::activate_signal(QConnectionList*, QUObject*) (in \
/usr/lib/qt3/lib/libqt-mt.so.3.3.7) ==11728==    by 0x5EF800C: \
QObject::activate_signal(int) (in /usr/lib/qt3/lib/libqt-mt.so.3.3.7) ==11728==    by \
0x5783BB8: KAction::activated() (in /opt/kde3/lib/libkdeui.so.4.2.0) ==11728==    by \
0x57B99E1: KAction::slotActivated() (in /opt/kde3/lib/libkdeui.so.4.2.0) ==11728==    \
by 0x588B09E: KAction::qt_invoke(int, QUObject*) (in /opt/kde3/lib/libkdeui.so.4.2.0) \
==11728==    by 0x5EF73CC: QObject::activate_signal(QConnectionList*, QUObject*) (in \
/usr/lib/qt3/lib/libqt-mt.so.3.3.7) ==11728==    by 0x5EF800C: \
QObject::activate_signal(int) (in /usr/lib/qt3/lib/libqt-mt.so.3.3.7) ==11728==    by \
0x5A28708: KAccelPrivate::menuItemActivated() (in /opt/kde3/lib/libkdecore.so.4.2.0) \
==11728==  Address 0x7CB7C64 is 4 bytes inside a block of size 32 free'd ==11728==    \
at 0x4022D21: operator delete(void*) (in \
/usr/lib/valgrind/x86-linux/vgpreload_memcheck.so) ==11728==    by 0x433B573: \
KMMsgInfo::~KMMsgInfo() (kmmsginfo.cpp:131) ==11728==    by 0x4386918: \
KMMsgList::set(unsigned, KMMsgBase*) (kmmsglist.cpp:100) ==11728==    by 0x43D1258: \
KMFolderMaildir::readMsg(int) (kmfoldermaildir.cpp:562) ==11728==    by 0x431F011: \
FolderStorage::getMsg(int) (folderstorage.cpp:476) ==11728==    by 0x42FCD48: \
KMFolder::getMsg(int) (kmfolder.cpp:310) ==11728==    by 0x42C18F7: \
KMHeaders::highlightMessage(QListViewItem*, bool) (kmheaders.cpp:2027) ==11728==    \
by 0x42C47D1: KMHeaders::finalizeMove(KMail::HeaderItem*, int, int) \
(kmheaders.cpp:1492) ==11728==    by 0x42C5B5B: KMHeaders::msgRemoved(int, QString) \
(kmheaders.cpp:1133) ==11728==    by 0x42C9846: KMHeaders::qt_invoke(int, QUObject*) \
(kmheaders.moc:301) ==11728==    by 0x5EF73CC: \
QObject::activate_signal(QConnectionList*, QUObject*) (in \
/usr/lib/qt3/lib/libqt-mt.so.3.3.7) ==11728==    by 0x42FE2BF: \
                KMFolder::msgRemoved(int, QString) (kmfolder.moc:254)
kmail: ########## KMFolder::find - mStorage->find( msg ) returns -1
kmail: ##### KMHeaders::applyFiltersOnMsg - Returned IDX is -1

Somehow it seems to be related to the message being selected before I use Ctrl-A \
Ctrl-J, but I'm not really sure about that.

Andreas


["kmail-3.5-branch.diff" (text/x-diff)]

Index: kmheaders.cpp
===================================================================
--- kmheaders.cpp	(revision 637361)
+++ kmheaders.cpp	(working copy)
@@ -1375,20 +1375,26 @@
         KPIM::BroadcastStatus::instance()->setStatusMsg( statusMsg );
         KApplication::kApplication()->eventLoop()->processEvents( QEventLoop::ExcludeUserInput, 50 );
       }
-      int idx = msgBase->parent()->find(msgBase);
-      assert(idx != -1);
-      KMMessage * msg = msgBase->parent()->getMsg(idx);
-      if (msg->transferInProgress()) continue;
-      msg->setTransferInProgress(true);
-      if ( !msg->isComplete() )
-      {
-        FolderJob *job = mFolder->createJob(msg);
-        connect(job, SIGNAL(messageRetrieved(KMMessage*)),
-                     SLOT(slotFilterMsg(KMMessage*)));
-        job->start();
-      } else {
-        if (slotFilterMsg(msg) == 2) break;
+      KMFolder *folder = msgBase->parent();
+      int idx = folder->find(msgBase);
+//      assert(idx != -1);
+      if ( idx >= 0 ) {
+		    KMMessage * msg = msgBase->parent()->getMsg(idx);
+		    if (msg->transferInProgress()) continue;
+		    msg->setTransferInProgress(true);
+		    if ( !msg->isComplete() )
+		    {
+			    FolderJob *job = mFolder->createJob(msg);
+			    connect(job, SIGNAL(messageRetrieved(KMMessage*)),
+						    SLOT(slotFilterMsg(KMMessage*)));
+			    job->start();
+		    } else {
+			    if (slotFilterMsg(msg) == 2) break;
+		    }
       }
+      else {
+        kdDebug (5006) << "##### KMHeaders::applyFiltersOnMsg - Returned IDX is " << idx << endl;
+      }
       progressItem->incCompletedItems();
     }
     progressItem->setComplete();
Index: folderstorage.cpp
===================================================================
--- folderstorage.cpp	(revision 637361)
+++ folderstorage.cpp	(working copy)
@@ -451,7 +451,8 @@
     if (msg->parent())
     {
       int idx = msg->parent()->find(msg);
-      take(idx);
+      if ( idx >= 0 )
+        take( idx );
     }
   }
 }
Index: kmfolder.cpp
===================================================================
--- kmfolder.cpp	(revision 637361)
+++ kmfolder.cpp	(working copy)
@@ -426,12 +426,20 @@
 
 int KMFolder::find( const KMMsgBase* msg ) const
 {
-  return mStorage ? mStorage->find( msg ) : 0;
+  if ( mStorage ) {
+    int idx = mStorage->find( msg );
+    if ( idx < 0 )
+      kdDebug (5006) << "########## KMFolder::find - mStorage->find( msg ) returns " << idx << endl;
+  }
+  else {
+    kdDebug (5006) << "########## KMFolder::find - mStorage is null: " << mStorage << endl;
+  }
+  return mStorage ? mStorage->find( msg ) : -1;
 }
 
 int KMFolder::find( const KMMessage* msg ) const
 {
-  return mStorage ? mStorage->find( msg ) : 0;
+  return mStorage ? mStorage->find( msg ) : -1;
 }
 
 int KMFolder::count( bool cache ) const


_______________________________________________
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