Fix a crash caused by invalid iterator (thanks to Michael Steinberg)

TOOL_MANAGER::finishTool() caused iterator to become invalid when an element was removed from the m_activeTools deque.
This commit is contained in:
Maciej Suminski 2016-07-04 16:12:21 +02:00
parent 4a20f376a3
commit 33e7fe6211
2 changed files with 23 additions and 6 deletions

View File

@ -489,9 +489,11 @@ optional<TOOL_EVENT> TOOL_MANAGER::ScheduleWait( TOOL_BASE* aTool,
void TOOL_MANAGER::dispatchInternal( const TOOL_EVENT& aEvent ) void TOOL_MANAGER::dispatchInternal( const TOOL_EVENT& aEvent )
{ {
// iterate over all registered tools // iterate over all registered tools
for( TOOL_ID toolId : m_activeTools ) for( auto it = m_activeTools.begin(); it != m_activeTools.end(); /* iteration is done inside */)
{ {
TOOL_STATE* st = m_toolIdIndex[toolId]; auto curIt = it;
TOOL_STATE* st = m_toolIdIndex[*it];
++it; // it might be overwritten, if the tool is removed the m_activeTools deque
// the tool state handler is waiting for events (i.e. called Wait() method) // the tool state handler is waiting for events (i.e. called Wait() method)
if( st->pendingWait ) if( st->pendingWait )
@ -507,7 +509,10 @@ void TOOL_MANAGER::dispatchInternal( const TOOL_EVENT& aEvent )
st->waitEvents.clear(); st->waitEvents.clear();
if( st->cofunc && !st->cofunc->Resume() ) if( st->cofunc && !st->cofunc->Resume() )
finishTool( st ); // The couroutine has finished {
if( finishTool( st, false ) ) // The couroutine has finished
it = m_activeTools.erase( curIt );
}
// If the tool did not request to propagate // If the tool did not request to propagate
// the event to other tools, we should stop it now // the event to other tools, we should stop it now
@ -635,8 +640,10 @@ void TOOL_MANAGER::dispatchContextMenu( const TOOL_EVENT& aEvent )
} }
void TOOL_MANAGER::finishTool( TOOL_STATE* aState ) bool TOOL_MANAGER::finishTool( TOOL_STATE* aState, bool aDeactivate )
{ {
bool shouldDeactivate = false;
// Reset VIEW_CONTROLS only if the most recent tool is finished // Reset VIEW_CONTROLS only if the most recent tool is finished
if( m_activeTools.empty() || m_activeTools.front() == aState->theTool->GetId() ) if( m_activeTools.empty() || m_activeTools.front() == aState->theTool->GetId() )
m_viewControls->Reset(); m_viewControls->Reset();
@ -648,10 +655,17 @@ void TOOL_MANAGER::finishTool( TOOL_STATE* aState )
aState->theTool->GetId() ); aState->theTool->GetId() );
if( tool != m_activeTools.end() ) if( tool != m_activeTools.end() )
{
shouldDeactivate = true;
if( aDeactivate )
m_activeTools.erase( tool ); m_activeTools.erase( tool );
} }
}
aState->theTool->SetTransitions(); aState->theTool->SetTransitions();
return shouldDeactivate;
} }

View File

@ -390,8 +390,11 @@ private:
* Deactivates a tool and does the necessary clean up. * Deactivates a tool and does the necessary clean up.
* *
* @param aState is the state variable of the tool to be stopped. * @param aState is the state variable of the tool to be stopped.
* @param aDeactivate decides if the tool should be removed from the active tools set.
* @return True if the tool should be deactivated (note it does not necessarily mean it has
* been deactivated, aDeactivate parameter decides).
*/ */
void finishTool( TOOL_STATE* aState ); bool finishTool( TOOL_STATE* aState, bool aDeactivate = true );
/** /**
* Function isRegistered() * Function isRegistered()