Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PerFrameHandler Framework Issue: Stuck Handler Prevents Execution of Subsequent Handlers #1602

Open
Nerexis opened this issue Sep 15, 2023 · 8 comments

Comments

@Nerexis
Copy link

Nerexis commented Sep 15, 2023

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 Nerexis added the Bug label Sep 15, 2023
@Nerexis
Copy link
Author

Nerexis commented Sep 15, 2023

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

@commy2
Copy link
Contributor

commy2 commented Sep 15, 2023

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

For example?

@PabstMirror
Copy link
Contributor

[{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
Copy link
Contributor

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

@Dystopian
Copy link
Contributor

but this doesn't belong in CBA

maybe add this as optional

@commy2
Copy link
Contributor

commy2 commented Sep 29, 2023

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

@Nerexis
Copy link
Author

Nerexis commented Oct 1, 2023

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

@jonpas
Copy link
Member

jonpas commented Apr 3, 2024

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 jonpas added Enhancement and removed Bug labels Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants