CMake project reloading.

Andreas Pakulat apaku at gmx.de
Wed Feb 3 07:48:27 UTC 2010


On 03.02.10 13:13:53, Olivier J. G. wrote:
> I've run into yet another bug that gets triggered by the reload that
> happens when CMakeLists gets updated.
> Create a class having a .cpp, and a .h, add it to CMakeLists.
> Select both the .cpp and .h, right-click, remove.
> Yes to remove from filesystem (for the .cpp, but it doesn't say)
> Yes to remove from CMakeLists
> Yes to the next remove from filesystem
> *Crash*
> When the cpp gets removed from the CMakeLists, cmake needs a reload,
> which deletes the ProjectItem representing the .h, main thread tries
> to remove the item, but it doesn't exist anymore.
> 
> I'm not just reporting a bug though, I know where to do that, and it's
> likely a dupe anyhow. The real problem is this whole CMakeLists folder
> doing a complete delete and reload cycle while other threads may still
> be using those ProjectItems. There was at least one other nasty bug
> that was caused by trying to access an item that was deleted by this
> reload cycle, and probably a few. Fixing this bug would only be
> working around the real problem...
> I can see why an edit to CMakeLists would require a reload, since
> there's no way to know if any of the items are still valid after a
> change to it, but it sounds like there should be some thread-locking
> for the use of ProjectItems so that things like this don't happen.

Hell no. I don't want another beast of the dimension of DUChain for the
project tree. David already suggested a viable alternative: Don't store
raw pointers to project items anywhere, except in the model itself (and
thats not our code). He suggested that everybody else only keep a struct
that has the path inside the model and when it tries to access something
it fetches the actual item from the model. That way you can do effective
null-pointer checks when actually using the item.

The other option would be using a ref-counted class instead of raw
pointers in the public API everywhere.

> Furthermore, in any case, I think the projectFileManager interface
> should expose removeFiles() rather than removeFile(). Otherwise you're
> talking about reloading the project for each and every .cpp that gets
> removed.

Well, actually I think the projectfilemanager should expose removeXXX
and deleteXXX separately. It matches more closely the add/create API and
allows to separate deletion from removal from project. And yes all of
them, except create, should be able to take a list of files/items.

> I would also think this should take a priority such that it gets done
> before release, because it's a load of crashing bugs that won't stop
> giving otherwise.

The steps you've described first are filed as bugreport already, the
exact same steps. Yet we don't have any duplicates of that particular
bug. There's the other way with the buildset trying to build an item
that has been removed from the tree due to reloading, but that also is
not hit very often.

So frankly, I don't see the need to invest the huge amount of work that
any proper solution to the problem imposes for 1.0. Its completely
inapropriate to start this at this point in the release cycle as a very
big change that will destabilize KDevelop quite a bit. Hence we'd have
to move the release date again and thats not something I'm willing to do
for a problem that only very few people run into.

Andreas

-- 
It's lucky you're going so slowly, because you're going in the wrong direction.




More information about the KDevelop-devel mailing list