Comment 7 for bug 1847532

So, I was thinking more about this and this actually is a result of some more insidious behavior in the tool framework/destructor methods.

The underlying issue is one of variable lifetime inside the tool stack. The cause of the crash was that we stack allocated an object that was passed to wxWidgets as a child element. Normally this will not be a problem because functions return and it gets destroyed then. But with our tool stack, the function never returned because it was waiting for a tool event and was not active. So essentially when the tool manager was deleted all its variables just hung around and no destructors were run and no stacks were cleared. This caused the crash since the object was not destroyed before we destroyed the window, so wxWidgets tried to destroy it. Since it was stack allocated, the call to delete was invalid and it crashed.

The fix I put together for this is to ensure that all active tools that have registered themselves on the tool stack of the frame get cancelled before the window deletion process really begins. This way the tools will exit gracefully and any stack allocated variables can be destroyed properly (this also will prevent memory leaks from these tools if they were active). I wanted to have the tool manager destructor do the stack clearance, but there is a lifetime issue with that as well. Since it is explicitly called in our highest-parent class (EDA_BASE_FRAME) all the destructors of the child classes have already run, which delete things like the board, schematic objects, etc. So any tools that are interacting with those objects would crash when we send the cancel event because they are trying to interact with already deleted objects. This meant I needed to put the stack clearance before we actually destroy any objects in the lowest-child class.

The other half of this issue that I just realized is our selection tool framework that is always running. Currently, there is no way to stop it, and so anything that is allocated on the stack before the while loop begins will never be destroyed. This shouldn't pose a problem currently because a quick glance into the tool main functions shows that they don't allocate any problematic items before they begin looping. We would get a problem though if we were to add any graphics objects (such as a status text popup like in the zone tool) to those tools in the same way as the zone tool. Then we would have the same variable lifetime issue as here.

The only good way to fix that would be to introduce a new tool event to end a tool (such as TA_END). This event could be sent in the destructor after the tool stack is cleared to ensure that the main tool loops are also stopped. Then we would have to include a break in the loop to handle that event (and modify the current assert( 0 ) in the function).

I don't know how likely we are to add anything more to the selection tool main function, so I don't know if that modification is going to be worth doing, but it is hard to predict what people will want to add to the functions in the future.