thecaptain
23-01-2008 06:22:37
For those of you who are having issues with crashing when using clearAll() in an event handler, I've found a workaround that seems to fix the problem.
The previous code in Widget::fireEvent() was:
The problem is that if the widget is destroyed while waiting for (*it)->execute(args) to return, the iterator will still try to increment (++it), but it will fail because it has been deleted.
I know the proper way to change states might be to set a flag that will call clearAll() on the next frame render, but I think this might introduce a race condition because on a multi-processor machine, an event could be in the process of being handled when the frame finishes rendering.
Anyway, I've found that if you replace the code with:
It works fine. To be correct, the code should actually have "break;" instead of "return true;" but the (mParentWidget != NULL) doesn't work properly when the parent widget is also destroyed.
edit: This actually has some problems, because the mUserEventHandlers has been deallocated at that if statement, so it's a crapshoot whether the memory has been used already. There has to be a better way... I tried exception handling, but catching asserts is something I'm not familiar with.
Perhaps the flag is the best option on the table right now....
The previous code in Widget::fireEvent() was:
// Execute registered handlers
EventHandlerArray* userEventHandlers = &(mUserEventHandlers[e]);
for( it = userEventHandlers->begin(); it != userEventHandlers->end(); ++it )
(*it)->execute(args);
if(mPropogateEventFiring[e] && (mParentWidget != NULL))
mParentWidget->fireEvent(e,args);
The problem is that if the widget is destroyed while waiting for (*it)->execute(args) to return, the iterator will still try to increment (++it), but it will fail because it has been deleted.
I know the proper way to change states might be to set a flag that will call clearAll() on the next frame render, but I think this might introduce a race condition because on a multi-processor machine, an event could be in the process of being handled when the frame finishes rendering.
Anyway, I've found that if you replace the code with:
// Execute registered handlers
EventHandlerArray* userEventHandlers = &(mUserEventHandlers[e]);
for( it = userEventHandlers->begin(); it != userEventHandlers->end(); ++it )
{
(*it)->execute(args);
// If this is true, the event must have deleted
// this event handler and should not continue firing.
if(userEventHandlers->size() == 0)
return true;
}
if(mPropogateEventFiring[e] && (mParentWidget != NULL))
mParentWidget->fireEvent(e,args);
It works fine. To be correct, the code should actually have "break;" instead of "return true;" but the (mParentWidget != NULL) doesn't work properly when the parent widget is also destroyed.
edit: This actually has some problems, because the mUserEventHandlers has been deallocated at that if statement, so it's a crapshoot whether the memory has been used already. There has to be a better way... I tried exception handling, but catching asserts is something I'm not familiar with.
Perhaps the flag is the best option on the table right now....