Fix RequestFrame

[DarthMan]

Hello. After some investigations regarding server crashes I discovered that the RequestFrame handles are not freed on map change. For example, if I use it inside the client_disconnected public forward it will cause memory leaks if the forward is called after pfnChangeLevel. It seems that the reason behind that is the fact the RequestFrame forwards are not freed at mapchange, like they are for most of the functions that return handles, such as register_forward. Manual deletion is also not possible, as the native doesn't return the handle that was created. Could this please be fixed in AMXX 1.10 ?

Thanks in advance! :)

[asherkin]

The root of the issue here seems to be that CSPForward stores a AMX * to identify the plugin to call the forward on, but that pointer is bound to the lifetime of the plugin - so if a plugin is unloaded then any CSPForward instances that still exist for it have a high chance of crashing when being executed.

[Th3-822]

i think that the issue is due to CFrameActionMngr not being cleared when everything is (and should be) cleared, manually adding a clear method seems to fix this:

diff --git a/amxmodx/CFrameAction.h b/amxmodx/CFrameAction.h
index ee368b6..3dceb7b 100644
--- a/amxmodx/CFrameAction.h
+++ b/amxmodx/CFrameAction.h
@@ -53,6 +53,13 @@ public:
        }
    }

+   void clear()
+   {
+       // Can't call deque's clear, so i just copied ExecuteFrameCallbacks
+       int callbacksToRun = m_requestedFrames.length();
+       while (callbacksToRun--) m_requestedFrames.popFront();
+   }
+
 private:
    ke::Deque<ke::AutoPtr<CFrameAction>> m_requestedFrames;

diff --git a/amxmodx/meta_api.cpp b/amxmodx/meta_api.cpp
index 81560a9..cd31a1e 100755
--- a/amxmodx/meta_api.cpp
+++ b/amxmodx/meta_api.cpp
@@ -393,6 +393,7 @@ int C_Spawn(edict_t *pent)

    }

+   g_frameActionMngr.clear();
    g_forwards.clear();

    g_log.MapChange();
@@ -771,6 +772,7 @@ void C_ServerDeactivate_Post()
    g_forcegeneric.clear();
    g_grenades.clear();
    g_tasksMngr.clear();
+   g_frameActionMngr.clear();
    g_forwards.clear();
    g_logevents.clearLogEvents();
    g_events.clearEvents();
@@ -1731,6 +1733,7 @@ C_DLLEXPORT   int Meta_Detach(PLUG_LOADTIME now, PL_UNLOAD_REASON reason)
    modules_callPluginsUnloading();

    g_auth.clear();
+   g_frameActionMngr.clear();
    g_forwards.clear();
    g_commands.clear();
    g_forcemodels.clear();

i want to make a PR with this fix, but i still have doubts if i can call deque's clear method directly from there, as is not on am-deque.h

[wilianmaique]

up?

[brunosilvaddr24]

Up?

[dvander]

@Th3-822 calling m_requestedFrames.clear() should be fine. Either way, your fix looks good.

[Th3-822]

@Th3-822 calling m_requestedFrames.clear() should be fine. Either way, your fix looks good.

after almost a year tried it but: /amxmodx/amxmodx/CFrameAction.h:64:21: error: no member named 'clear' in 'ke::Deque<ke::AutoPtr<CFrameActionMngr::CFrameAction>>' m_requestedFrames.clear();

@dvander should i try to add a clear method in amtl/am-deque.h and do a PR there ? or just keep the workaround without it?

edit: looks like it's using a old amtl commit and it's too hard to read for me, so i think i should just keep the workaround instead