Comment 15 for bug 1847532

Revision history for this message
Ian McInerney (imcinerney) wrote :

@Seth, the main issue is not with the tool manager itself but with the tools that are running. I agree that the pointer loop is not ideal, and I was actually thinking that maybe turning the tool manager into something that the child frames inherit from instead of instantiate could be a fix for that (that is how wxWidgets handles their event handler routines). Then the child frames inherit from both the EDA_BASE_FRAME chain and the TOOL_MANAGER class. We then document that the tool manager class is for inheritance only and is never to be directly instantiated. This should allow us to just use "this" as the frame pointer inside the tool manager routines.

Removing the pointer loop between the frame and the tool manager doesn't solve the issue with a tool referencing an object from the frame. For instance, look at the pcbnew drawing tool's DrawDimension action. After the while loop runs, the tool calls into the view controls and also into the frame (and passes the board object). This means that we will need to be careful when stopping this tool, because if the part after the loop is run after we delete the board, there will be an issue.

An alternative would be to introduce the TA_END/STOP action and have the loop immediately check for it and return from inside the loop so that no other code can be run. I don't know if that will fix everything though, because what if the developer instantiates an object on the stack whose destructor references a frame element? I think that would still get us into trouble. While I agree it would be best if the tool manager destructor could shutdown the tools, I don't know if that is really feasible given that a tool can do basically anything (and that we don't know what kind of tools we might want in the future). It seems the safest option is to shutdown all the tools when we first begin the destuction of the frames (similar to how an OS stops all user programs before it stops the kernel). Yes it will require an explicit call in each frame destructor, but then I think we can guarantee we don't get into possible lifetime issues.