Crash after requesting context menu (after reopening the project)

[Manuel Breugelmans]

On Thursday 31 July 2008 11:22:03 Andreas Pakulat wrote:
> On 31.07.08 10:31:48, Manuel Breugelmans wrote:
> > On Thursday 31 July 2008 08:39:29 Andreas Pakulat wrote:
> > > On 31.07.08 01:59:24, Manuel Breugelmans wrote:
> > > > On Wednesday 30 July 2008 00:43:17 Andreas Pakulat wrote:
> > > > > Hmmm... I can't see how this could happen. Granted the project
> > > > > pointer itself isn't deleted until the event loop runs again after
> > > > > closing it, but the project items are being deleted properly. So
> > > > > the project tree has to have the new items and their project()
> > > > > pointer needs to be "proper".
> > > > >
> > > > > You should be able to debug this by simply printing out the pointer
> > > > > of the project in projectcontroller::closeProject() before its
> > > > > deleted and then also in veritas coverageplugin before the crash
> > > > > line. Those two pointers should be different. If they are, there
> > > > > must be a problem elsehwere.
> > > >
> > > > I've given this a long hard look and spotted the culprit. Not dvcs
> > > > nor veritas but some obscure plugin loading code in projectcontroller
> > > > [ the person that introduced this now owes +1 beer to Evgeniy & me :)
> > > > ]
> > > >
> > > > I deduced this with the following minimal test cases:
> > > > */ start kdev4 >> load project >> reopen project [no close, just
> > > > recent-
> > > >
> > > > >project] >> context menu >> crash
> > > >
> > > > */  start kdev4 >> load project >> close project >> load project >>
> > > > context menu >> no crash
> > > > */ start kdev4 >> load cmake project >> load other cmake project >>
> > > > reload cmake project >> context on reloaded >> no crash
> > > > */ start kdev4 >> load cmake project >> load make project >> reload
> > > > cmake project >> context menu reloaded >> crash
> > > >
> > > > On a reopen project ProjectController unloads the KDevCmakeBuilder
> > > > plugin (IF no other open projects use it) then immediatly loads it
> > > > again, which results in that segfault.
> > >
> > > Did you find out why?
> >
> > I'm not 100% but I think it's the plugin doing a deleteLater() when
> > unloading.
>
> Aaah, now I see whats happening. The problem is that the
> plugincontroller only removes a plugin from its internal cache when its
> actually destroyed, not when its unloaded. Thats wrong and needs to
> change. Going to do that right now.

sweet

> > > > 2. Refactor the ProjectController->closeProject() member as this had
> > > > started to rot
> > >
> > > No it didn't. How do you know it rots? I'm not seeing much sense behind
> > > factoring out code from closeProject, unless its used in more than one
> > > place - which isn't the case (taking into account the above comment).
> >
> > Hmm that's common practice to increase readability, or so I was taught ;)
> > Whenever you feel like a chunk of code is non trivial enough to comment
> > it, use an ExtractMethod refactoring instead. I would say keep that.
>
> Well, I don't think this code is fitting into that. Its pretty straight
> forward IMHO.
>

There's plenty of comments there though. The thing is that you want code to be 
readable for people not familiar with a particular component, like I was. If 
they think it is not reading fluently, it should be re-factored. 
'unloadUnusedPlugins()' shows much better what is going on and allows a reader 
to skip it if that's not what they are looking for, rather than getting 
overwhelmed with details.


Manuel