[Bug 221109] Crash when deleting multiple folder [ProjectManagerViewPlugin::removeFolderFromContextMenu, ProjectManagerViewPlugin::qt_metacall, QMetaObject::metacall]

[Milian Wolff]

On Sunday 03 January 2010 21:12:46 Andreas Pakulat wrote:
> I'm moving this to the devel list as thats a better place to discuss
> technical details.
> 
> On 03.01.10 18:32:52, Milian Wolff wrote:
> > But you are aware that the watcher is the culprit here and that this
> > would fix the crash(es)?
> 
> So the problem is:
> 
> 1. User selects multiple cmake-known dirs and selects "delete"

1b) the projectmanagerview deletes the first folder

> 2. User gets to the cmake edit dialog, because the first item is about
> to be removed
> 3. User deletes multiple entries from the cmake files
> 4. CMake stores the new file, which triggers the watcher, which reloads
> the file which deletes the items from the model
> 5. the next item is tried to be removed, but it doesn't exist anymore

5. is again the projectmanagerview, not the cmakemanager.


But other than that: You got to the point.

> So the first question is: Why is the watcher active for a file that is
> being saved by us. Or rather, why do we reload it if we just saved it. I
> had a similar problem at work and the solution was to simply store the
> modification time right after the save and only reload if the
> modification time is newer than this stored one. That saves all these
> kind of problems.

You mean one should temporarily disable the watcher before filesave? But we 
still have to reparse the "new" cmakefile and that will (currently) lead to a 
reload of the cmake project's model as far as I saw in the sources. Hence 
_all_ item's (below a given folder) get invalidated.

> Another thing is that the cmake support should save the cmake file only
> once he deleted all project items, not after deleting the first one or
> even before that.

Yes, that would work as well of course, i.e. passing a QList of items. If the 
project gets reloaded after all changes got applied, these things should work. 
Though of course this does not guard against behind-the-back changes (VCS 
updates, manual deletion, ....) in circumstances like I described in the 
bugreport (i.e. deletion while a dialog is shown). There only refcounting or 
globally disabling the watcher for that period would help.

> Just IMHO of course and I didn't look at the code!
> 
> > Making them ref-counted would remove the crash but would still be
> > confusing to the User:
> >
> > - delete first
> > - in wizard delete others
> > - all other delete actions will trigger error messages (since the folders
> > are not found and hence cannot be deleted etc.)
> 
> No that wouldn't happen, the items would still be deleted as either the
> watcher or the real deleteFolder code would do that. Just that either
> can check wether the item has already been deleted or not.

What would probably make much more sense would be a designated dialog that 
lists all directories/files that will/should get deleted with tickboxes to 
enable/disable the action. Having a dialog popup once for every folder is very 
combersome imo. Then the ApplyChangesWidget could handle multiple files and 
only save them all in one go (with deactivated watcher) and reanble the 
watcher afterwards and reparse the top-most dirs.
-- 
Milian Wolff
mail at milianw.de
http://milianw.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20100104/c7320afd/attachment.sig>