CBA_A3 icon indicating copy to clipboard operation
CBA_A3 copied to clipboard

PerFrameHandler Framework Issue: Stuck Handler Prevents Execution of Subsequent Handlers

Open Nerexis opened this issue 1 year ago • 8 comments

Mods (complete and add to the following information): N/A

Description: The perFrameHandler framework in CBA_A3 mod for ArmA 3 is causing a problem where, if one of the perFrameHandlers gets stuck or takes too long to complete, the other handlers in the list do not get called. This results in some event handlers never being executed. https://github.com/CBATeam/CBA_A3/blob/master/addons/common/init_perFrameHandler.sqf

Steps to reproduce: Launch ArmA 3 with the CBA_A3 mod enabled. Join a game where perFrameHandler event handlers are used (can be a custom mission or mod scenario that heavily relies on event handlers). Introduce a perFrameHandler that has a long execution time or gets stuck (e.g., infinite loop). Observe that other perFrameHandlers do not get executed.

Expected behavior: Each perFrameHandler should execute independently of the others, so if one is stuck or takes too long, it should not prevent the others from being executed. Not sure how to fix this, maybe by introducing timeouts or at least better info if one of handlers is taking too long.

Where did the issue occur?

  • Dedicated / Self-Hosted Multiplayer / Singleplayer / Editor (Singleplayer) / Editor (Multiplayer) / Virtual Arsenal

Log Files: N/A

Additional context: The architecture's current design of iterating through a list to call each handler means that if one gets stuck, the others are prevented from being called. This is particularly problematic in complex scenarios or mods where multiple event handlers are crucial for the mission or gameplay.

    {
        _x params ["_function", "_delay", "_delta", "", "_args", "_handle"];

        if (diag_tickTime > _delta) then {
            _x set [2, _delta + _delay];
            [_args, _handle] call _function;
        };
    } forEach GVAR(perFrameHandlerArray);

Screenshots: N/A

Nerexis avatar Sep 15 '23 10:09 Nerexis

Solution can be based on my code here: https://pastebin.com/f7LDJxGv

Nerexis avatar Sep 15 '23 11:09 Nerexis

Introduce a perFrameHandler that has a long execution time or gets stuck (e.g., infinite loop).

For example?

commy2 avatar Sep 15 '23 13:09 commy2

[{systemChat format ["A: %1", time];}, 0, []] call CBA_fnc_addPerFrameHandler; 
[{5 == []}, 0, []] call CBA_fnc_addPerFrameHandler; 
[{systemChat format ["C: %1", time];}, 0, []] call CBA_fnc_addPerFrameHandler;

C never runs

PabstMirror avatar Sep 15 '23 16:09 PabstMirror

addMissionEventHandler ["ScriptError", {
    params ["_errorText", "_sourceFile", "_lineNumber", "_errorPos", "_content", "_stackTraceOutput"];
    private _src = _stackTraceOutput # 0;
    if ((_src#0 == "x\cba\addons\common\XEH_postInit.sqf") && {(_src#1 == 5)}) then {
        private _pfID = (_stackTraceOutput#2#3) get "_handle";
        systemChat format ["PFEH %1 has failed and will be removed", _pfID];
        diag_log text format ["PFEH %1 has failed and will be removed", _pfID];
        [_pfID] call CBA_fnc_removePerFrameHandler;
    };
}];

you can add that code and it should catch any failing PFEHs and remove them

but this doesn't belong in CBA because it could break things by removing a PFEH that only fails once we could use a isNil wrapper but that has a perf penalty ultimately the best solution is to not have code that can error

PabstMirror avatar Sep 23 '23 03:09 PabstMirror

but this doesn't belong in CBA

maybe add this as optional

Dystopian avatar Sep 23 '23 08:09 Dystopian

Code that breaks stuff should not be endorsed, even by being optional. Everything works as expected as is.

commy2 avatar Sep 29 '23 04:09 commy2

Could we consider making this code more resilient by optionally adding an isNil check (switch in Addons options?)? This would help in scenarios where it's crucial to avoid crashes and ensure continuity of operation, much like landing a satellite on the moon. A failure in one PFH shouldn’t compromise the entire landing operation (high-stakes, high-reliability scenarios).

Nerexis avatar Oct 01 '23 10:10 Nerexis

Could we consider making this code more resilient by optionally adding an isNil check (switch in Addons options?)? This would help in scenarios where it's crucial to avoid crashes and ensure continuity of operation, much like landing a satellite on the moon. A failure in one PFH shouldn’t compromise the entire landing operation (high-stakes, high-reliability scenarios).

I agree with this, as long as it doesn't impact performance. We could error on one PFH and keep running the rest.

jonpas avatar Apr 03 '24 14:04 jonpas