Fix heap use after free in MoveTool

[J5lx]

Steps to reproduce:

• Use something like AddressSanitizer which detects heap use after free
• Open Pencil2D
• Switch to move tool
• Move cursor across ScribbleArea
• Ctrl-N to create new project
• heap use after free occurs in MoveTool::leavingThisTool because the move tool keeps an attribute with a pointer to the last used layer which it tries to access after the old project is already unloaded.

Solution: get rid of the pointer.

I also noticed the same kind of layer pointer attribute in SelectTool. Although I don’t think it’s causing any issues right now I removed it anyway because the layer can just as easily passed as a parameter there and storing it as an attribute basically just serves as a booby trap for the next person making changes to that tool.

[MrStevns]

I can confirm that the bug is reproducible and has been fixed with the changes in the PR.

Feel free to merge when you're ready 👍

Discussion

While it's fine to remove pointers in this case, I think in general that we need to figure out a way to cleanup old state when loading a new file and such.

One way could be to make all tools listen to Editor::objectLoaded and cleanup all state, like it was a de-initialization, however that's still very manual and error prone.

We also have ToolManager::cleanupAllToolsData although that's seemingly unused right now.

Another would be to initialize all tools again, that's a small overhead for the convenience of having a clean slate, and it only needs to be done in the code once to take effect.

What do you think?

[J5lx]

What do you think?

I agree something needs to be done about that. That said, right now I don’t really have a preference for any of the suggestions you made. I also think there is a larger problem here – the fact that the program relies so much on global state and much of it is so strongly coupled. (I’m reminded of how we can’t even do a command-line export without creating a main window first because otherwise the program just crashes.) If we work on that, I reckon it will also at the very least alleviate issues like this. But of course, it’s a bigger undertaking and there’s something to be said about making smaller improvements first without waiting for larger projects that may take a long time to materialise. For now, I don’t really have any conclusive thoughts on your suggestions.