Add OnNotifyPluginUnloaded() forward

[Wend4r]

Issue #890

I wrote this Pull Request in considerations of the handles principles that a plugin must necessarily know when the handle closes or close manually if it is cloned.

[Wend4r]

Code to check

pull1462.sp

#pragma semicolon 1
#pragma newdecls required

#include <sourcemod>

#define MAX_Cycle_MESSAGE_LENGTH 1024

enum struct CycleMessageInfo
{
	Handle plugin;
	char message[MAX_Cycle_MESSAGE_LENGTH];

	void SendMessage()
	{
		PrintToServer("%s", this.message);
	}
}

ArrayList g_CycleMessageList;

public APLRes AskPluginLoad2(Handle myself, bool late, char[] error, int err_max)
{
	CreateNative("SendCycleMessage", Native_SendCycleMessage);
}

any Native_SendCycleMessage(Handle plugin, int numParams)
{
	CycleMessageInfo info;

	info.plugin = plugin;

	LogMessage("Detected %X plugin", plugin);

	GetNativeString(1, info.message, sizeof(CycleMessageInfo::message));
	g_CycleMessageList.PushArray(info);

	HookPluginUnload(plugin, OnPluginUnloaded);
}

public void OnPluginStart()
{
	g_CycleMessageList = new ArrayList(sizeof(CycleMessageInfo));

	CreateTimer(5.0, OnPrintMessageTimer, _, TIMER_REPEAT);
}

Action OnPrintMessageTimer(Handle timer)
{
	CycleMessageInfo info;

	for(int i = 0, length = g_CycleMessageList.Length; i != length; i++)
	{
		g_CycleMessageList.GetArray(i, info, sizeof(info));
		info.SendMessage();
	}
}

void OnPluginUnloaded(Handle plugin)
{
	LogMessage("Plugin %X is unloaded", plugin);

	int index = g_CycleMessageList.FindValue(plugin, CycleMessageInfo::plugin);

	g_CycleMessageList.Erase(index);
}

pull1462_module.sp

#pragma semicolon 1
#pragma newdecls required

#include <sourcemod>

native void SendCycleMessage(const char[] message);

public void OnPluginStart()
{
	SendCycleMessage("Hello!");
}

Result:

[KyleSanderson]

@dvander we've been all over the map on this one over the years - what's your take? I need it for memory management in another plugin but that usecase is very different.

[dvander]

The idea behind this seems good to me, but there are some complexity and stability issues here.

I think you could fix them by removing PluginUnloadHookInfo. If you put the IChangeableForward directly on the CPlugin object, this patch becomes a lot simpler, and you won't have the stability issues (IChangeableForward handles the complexity of list iteration for you).

[Wend4r]

Works!

source.zip

sm plugins load pull1462
[SM] Loaded plugin pull1462.smx successfully.
sm plugins load pull1462_module
L 05/24/2021 - 23:31:18: [pull1462.smx] Detected 27800CC plugin
[SM] Loaded plugin pull1462_module.smx successfully.
Hello!
Hello!
Hello!
sm plugins unload pull1462_module
L 05/24/2021 - 23:31:34: [pull1462.smx] Destroyed 27800CC plugin
[SM] Plugin pull1462_module.smx unloaded successfully.

[dvander]

Thanks! This is much simpler. Just one small comment.

[KyleSanderson]

This looks slightly bugged (the forward says Unloaded, but the plugin is still loaded). I think we may need two forwards here, one for OnPluginUnloading, and a final for OnPluginUnloaded after everything has released resources.

[dvander]

I think this is fine and we can merge it.

Once "OnPluginEnd" has been called, it is for all intents and purposes unloaded, and the only thing left to do is cleanup. Besides not having a clear use case, what you're asking for has a catch-22: if you release the resources, you don't have the handle anymore, so how can you notify?

[KyleSanderson]

Once "OnPluginEnd" has been called, it is for all intents and purposes unloaded, and the only thing left to do is cleanup.

For my specific usecase Forwards are destroyed in OnPluginUnloaded, so we need a final call for OnPluginDestroyed (or the equivalent of it) in pawn. Wendr is looking for the same behaviour it looks like here, because they're using the plugin handle as a value to be found in an array. When you'd use OnPluginUnloading is if you need the pubinfo from the plugin, or to access resources still allocated which would match the OnClientDisconnecting vs OnClientDisconnected behaviour, no?

[Wend4r]

This looks slightly bugged (the forward says Unloaded, but the plugin is still loaded). I think we may need two forwards here, one for OnPluginUnloading, and a final for OnPluginUnloaded after everything has released resources.

In my case, this forward is needed to clean up the data attaches to the native calling handler at the SourcePawn level. I do not know in which cases an additional forward for a fully cleaned plugin from extensions (IPluginsListener) be useful.

[dvander]

"For my specific usecase Forwards are destroyed in OnPluginUnloaded" - This doesn't explain why such a thing would be needed, and indeed there is nothing within SourceMod that needs this distinction internally.

[KyleSanderson]

Thanks for bearing with us team, this has now landed (if there are other issues related to this please let them know).

#948
#1482
#1481

[dragokas]

That commit does not cover OnPluginLoad(), which is now closed: #1482