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

List:       kde-pim
Subject:    Re: [Kde-pim] Review Request 109259: Remove inode/directory from MailCommon::FolderCollectionMonitor
From:       "Andras Mantia" <amantia () kde ! org>
Date:       2013-03-05 15:01:35
Message-ID: 20130305150135.20007.70332 () vidsolbach ! de
[Download RAW message or body]



> On March 3, 2013, 6:14 p.m., Wolfgang Rohdewald wrote:
> > > fix a problem, that removing a completely unrelated resource (like Google Calendar
> > > was totally messing up KMail's folder view (usually leading to crash).
> > 
> > How does it fix that? By not going thru the code path which makes kmail crash? But then the \
> > crashing bug is still there, it is just not triggered. Is it safe to assume that this \
> > dangerous code path is never taken elsewhere? To me, a bug messing up the folder view and \
> > crashing seems to be worthy of its own investigation, but you are removing one (although \
> > maybe the only one) way how to trigger it.
> 
> Dan Vrátil wrote:
> I believe the crash is caused by one of the proxy models which are on top of the \
> EntityTreeModel. When collections owned by the removed resource are removed from to model, \
> the proxy model does not handle that, because it does not expect collections with non-KMIME \
> content to be in the model. 
> Dan Vrátil wrote:
> *...are removed from the root model...
> 
> Wolfgang Rohdewald wrote:
> so could the proxy model be changed to always ignore items with non-KMIME content? (I say \
> items and not collections because I do not know if it is allowed to mix different types of \
> content within the same collection) 
> Dan Vrátil wrote:
> I don't see any reason why a KMail-specific proxy model should handle collections that don't \
> contain emails... 
> Volker Krause wrote:
> What's actually causing the crash still needs to be analyzed I think, but even if this just \
> hides a bug in a proxy model down the line, the fact that KMail was loading many more \
> collections than actually relevant for it is wrong by itself already. So, this patch is \
> correct IMHO. 
> Wolfgang Rohdewald wrote:
> I fully agree - I never meant to say anything against this patch by itself. Only the fact \
> that this makes it improbable the crash reason will ever be analysed worries me a bit. For me \
> crashes have a much higher priority than optimizations. Is there a bug report about that \
> crash? Could it be augmented with a hint that this patch hides it?

Might be unrelated (I don't know what the crash is exactly here), but at the sprint I could \
fairly easily crash KMail by repeating "create folder", "delete folder" operations on an IMAP \
resource. After a few cycles, it crashes (well, asserted) in the ETM.


- Andras


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109259/#review28487
-----------------------------------------------------------


On March 3, 2013, 2:59 p.m., Dan Vrátil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109259/
> -----------------------------------------------------------
> 
> (Updated March 3, 2013, 2:59 p.m.)
> 
> 
> Review request for KDEPIM and Laurent Montel.
> 
> 
> Description
> -------
> 
> Don't watch collections in KMail that KMail is not really interested in. This should improve \
> performance a bit and fix a problem, that removing a completely unrelated resource (like \
> Google Calendar) was totally messing up KMail's folder view (usually leading to crash). 
> I assume this was added as a workaround for a bug in Monitor, that collections trees where \
> the root collection did not match the mimetype filter were ignored. This has been fixed some \
> time ago, so this is not needed anymore. 
> 
> Diffs
> -----
> 
> mailcommon/foldercollectionmonitor.cpp 49de537 
> 
> Diff: http://git.reviewboard.kde.org/r/109259/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Vrátil
> 
> 

_______________________________________________
KDE PIM mailing list kde-pim@kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/


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