Common - Fix potential leak on cachedCall

[LinkIsGrim]

When merged this pull request will:

• Fix cachedCall adding a cache to its event's clear list on every recompute rather than once. The list is only emptied when the event fires, so a cache whose expiry is shorter than the gap between firings adds an entry per expiry until it does.

Would leak when short expiry combined with rare event. Not the case for existing usages but #11450 may hit it.

IMPORTANT

• If the contribution affects the documentation, please include your changes in this pull request so the documentation will appear on the website.
• Development Guidelines are read, understood and applied.
• Title of this PR uses our standard template Component - Add|Fix|Improve|Change|Make|Remove {changes}.

[johnb432]

ELI5?

[LinkIsGrim]

cached call recomputed and re-added itself to the list of caches that event clears instead of only doing that once

list only gets reset when the event fires, so if the cache expires faster than the event fires, the list just keeps growing with duplicates forever until the event actually fires

fix makes it store what the cache was last listed on alongside the cache itself, so it only re-adds itself to an event's list when that set of events actually changes. if it's already listed then recompute skips listing.

[johnb432]

I now see the issue, but there are more I also see:

• With your fix, the issue could still happen if you alternate between different events e.g. ["test1"] and ["test2"], no?
• If you reuse an old UID that used events, but in a new call you don't, the events will still clear it afaik.

---

The more I look at this code, the more I see issues. I'm looking into rewriting it completely.

[johnb432]

params ["_params", "_function", "_namespace", "_uid", "_duration", ["_events", []]];

// GVAR(clearCache) = createHashMap; // would be defined in pre or post init

_uid = QGVAR(uid) + _uid;

(_namespace getVariable [_uid, [-99999]]) params ["_expiry", "", ["_listedOn", []]];

// Using a hashmap here might be more efficient, but wouldn't delete cached result if namespace is deleted (especially relevant for objects)
if (_expiry < diag_tickTime) then {
    // Does the cache need to be cleared on an event?
    if (_events isNotEqualTo [] && _events isNotEqualTo "") then {
        if (_events isEqualType "") then {
            _events = [_events];
        };

        {
            private _cacheList = GVAR(clearCache) getOrDefault [toLower _x, []];

            if (_cacheList isEqualTo []) then {
                continue;
            };

            _cacheList deleteAt (_cacheList find [_namespace, _uid]);
        } forEach (_listedOn - _events);

        _listedOn = _events - _listedOn;

        {
            private _event = toLower _x;
            private _cacheList = GVAR(clearCache) get _event;

            // If there was no EH to clear these caches, add one
            if (isNil "_cacheList") then {
                _cacheList = [];
                GVAR(clearCache) set [_event, _cacheList];

                [_event, {
                    #ifdef DEBUG_MODE_FULL
                        INFO_1("Clear cached variables on event: %1",_eventName);
                    #endif
                    // Get the list of caches to clear
                    //IGNORE_PRIVATE_WARNING ["_eventName"];
                    // _eventName is defined on the function that calls the event
                    private _event = toLower _eventName;
                    // Erase all the cached results
                    {
                        _x call FUNC(eraseCache);
                    } forEach (GVAR(clearCache) getOrDefault [_event, []]);
                    // Empty the list
                    GVAR(clearCache) set [_event, []];
                }] call CBA_fnc_addEventHandler;
            };

            // Add this cache to the list of the event
            _cacheList pushBackUnique [_namespace, _uid];
        } forEach _listedOn;
    };

    _namespace setVariable [_uid, [diag_tickTime + _duration, _params call _function, _listedOn]];

#ifdef DEBUG_MODE_FULL
    INFO_2("Calculated result: %1 %2",_namespace,_uid);
} else {
    INFO_2("Cached result: %1 %2",_namespace,_uid);
#endif

};

(_namespace getVariable _uid) select 1

This handles (at least in theory) different events used for clearing caches - thoughts?