amxmodx
amxmodx copied to clipboard
Fix RequestFrame
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! :)
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.
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
up?
Up?
@Th3-822 calling m_requestedFrames.clear()
should be fine. Either way, your fix looks good.