Issues with clearAll()

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:

// 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....

Exception handling won't be of any use here, because you will always run into undefined behaviour. Using your flag method is an option. I can't really think of anything better right now.

But there the problem doesn't stop. If you destroy a single widget from its event handler, you should get similar problems. Maybe the whole widget deletion process must be reviewed.

There is a GUIManager::mFreeList which handles deferred deleting of objects, but I am not sure, if it is of any help here (the code is a bit complex and I am a bit in a hurry, so I can't take a closer look at it right now).

This problem existed way back when I was using CEGUI. The only workaround I could find was to set a state flag and check it later. You can work around race conditions by checking if the previous state has been fully unloaded. One state should not load until the previous state is unloaded.

You should avoid deleting a widget inside an event handler.

Yeah, the flag method sounds good. I think I might add a Mutex accessor so you can make sure that an event is not being processed when the widgets are being removed. Then there wouldn't be a problem.