gx_studio_auto_event_handler calls event function on a deleted widget
Closed this issue · 9 comments
I am using the Screen Flow in GUIX Studio.
I have configured 2 Actions for a Runtime Allocated widget when a button is pressed.
- Detach itself
- Attach another widget
This seems to work most times fine but sometimes not!
To me it looks like there is a problem in the auto generated gx_studio_auto_event_handler function.
In this function all the GUIX Studio Actions are handled in a switch case .
I debugged into the case GX_ACTION_TYPE_DETACH and I can see that the widget is detached gx_widget_detach(target) and also deleted gx_widget_delete(target).
Now the problem I think is that still the event function of that widget is called at the end status = record->chain_event_handler(widget, event_ptr);
The widget might contain random data at that point. Most times it works fine because of the checks in the "_gxe..." functions, but sometimes there is random valid data in that widget and then i get a null pointer exception.
@Serpet-coder Try to add the following code to the relevant gxe function to check invalid widget and see if this change solve the problem.
if (button-> gx_widget_type == 0)
{
return(GX_INVALID_WIDGET);
}All the gxe functions have that check already.
Here is a code snippet which seems to fix the problem. I made some changes to the auto-generated function gx_studio_auto_event_handler(). (But since it's auto-generated it will be overwritten next time i generate)
My changes basically delay detach/delete operations on the widget itself until the end of the function (after all actions are handled). I think the problematic cases are GX_ACTION_TYPE_DETACH and GX_ACTION_TYPE_TOGGLE where we delete the widget itself.
The event processing function of a detached/deleted widget won't be called anymore with this change.
UINT gx_studio_auto_event_handler(GX_WIDGET *widget, GX_EVENT *event_ptr, GX_CONST GX_STUDIO_EVENT_PROCESS *record)
{
UINT status = GX_SUCCESS;
GX_CONST GX_STUDIO_ACTION *action;
GX_CONST GX_WIDGET *parent = GX_NULL;
GX_WIDGET *target = GX_NULL;
GX_CONST GX_STUDIO_EVENT_ENTRY *entry = record->event_table;
GX_BOOL detach_self = GX_FALSE; // remember to detach ourself
GX_BOOL delete_self = GX_FALSE; // remember to delete ourself
GX_WIDGET *toggle_target = GX_NULL; // remember the toggle target
while(entry->event_type)
{
if (entry->event_type == event_ptr->gx_event_type)
{
if((entry->event_type == GX_EVENT_ANIMATION_COMPLETE) &&
(entry->event_sender != event_ptr->gx_event_sender))
{
entry++;
continue;
}
action = entry->action_list;
while(action->opcode)
{
switch(action->opcode)
{
...
case GX_ACTION_TYPE_DETACH:
target = gx_studio_action_target_find(widget, action);
if (target)
{
if(widget == target)
{
detach_self = GX_TRUE; // we might need our parent for another action, so don't detach yet
}
else
{
gx_widget_detach(target);
}
if (target->gx_widget_status & GX_STATUS_STUDIO_CREATED)
{
if(widget == target)
{
delete_self = GX_TRUE; // we might need widget data for another action, so don't delete yet
}
else
{
gx_widget_delete(target);
}
}
}
break;
case GX_ACTION_TYPE_TOGGLE:
if(action->flags & GX_ACTION_FLAG_POP_TARGET)
{
gx_system_screen_stack_get(GX_NULL, &target);
}
else
{
target = gx_studio_action_target_get(widget, action);
}
// gx_studio_screen_toggle(widget, target);
toggle_target = target; // we might need widget data for another action, so don't toggle yet
break;
...
default:
break;
}
action++;
}
}
entry++;
}
if(toggle_target)
{// all actions are handled now we can toggle
gx_studio_screen_toggle(widget, toggle_target);
}
else if(detach_self)
{// all actions are handled now we can detach and delete
gx_widget_detach(widget);
if(delete_self)
{
gx_widget_delete(widget);
}
}
else if (record->chain_event_handler)
{
status = record->chain_event_handler(widget, event_ptr);
}
return status;
}@Serpet-coder The first parameter widget is normally a top level screen. The detached widget might be a child of the widget. So, in some case, this might not work, as we the chained event handler not been called once there is a child widget being detached. I have checked that some gxe functions do not have the widget type check, for example, gxe_button_event_processing is missing this checking. If a widget has been deleted, the widget type is set to 0, if the gxe event processing function does the widget type checking, it will return directly.
Isn't problem here that if you have a GX_STATUS_DYNAMICALLY_ALLOCATED widget. When it is deleted the widget is freed. Your check would then dereference an invalid adress somewhere on the heap.
@Serpet-coder Thanks for your feedback! I'll investigate the issue and keep you informed of any progress.
@Serpet-coder A new version of GUIX Studio 6.3.0.1 has been released to the Microsoft App Store that addresses the mentioned issue. Please let me know if the fix has solved your problem.
Thank you @ting-ms!
I have tried GUIX Studio 6.3.0.1.
This helps a bit but i think it is still not optimal for 2 reasons:
- In case
GX_ACTION_TYPE_TOGGLEthe widget will get deleted too -> So this the exact same problem asGX_ACTION_TYPE_DETACH - If the widget gets deleted the event handler will still be called but now with a NULL pointer. This would require me to add a NULL pointer check into ALL of my event functions.
I thinkrecord->chain_event_handler(widget, event_ptr)should never be called ifwidget == GX_NULL
@Serpet-coder Thank you for your advice! We'll continue refining the fix and keep you informed.
@Serpet-coder A new version of GUIX Studio 6.4.0.0 has been released to the Microsoft App Store that addresses the additional issues you mentioned. If you encounter any further issues, please feel free to reopen it.