Zeex / samp-plugin-crashdetect

Crash/error reporting plugin for SA-MP server
http://forum.sa-mp.com/showthread.php?t=262796
BSD 2-Clause "Simplified" License
116 stars 23 forks source link

Compatibility with AMX machines created on-the-fly #61

Closed IS4Code closed 6 years ago

IS4Code commented 6 years ago

Hi! I am testing PawnPlus with the latest versions of CrashDetect, and while it works mostly fine with the more "daring" features of the plugin, there is a crash in HandleAMXExecError for newly forked AMX machines, which didn't happen in 4.15 I used before.

The actual issue lies in the call to CrashDetectHandler::GetHandler which returns nullptr for AMX instances that are not registered. When I produce a temporary clone of an AMX machine, I cannot inform other plugins about it, because I can't call AmxLoad in them without relying on memory addresses, which I want to avoid, and since CrashDetect hooks all AMX calls, it is provided with an unregistered AMX.

An easy fix is to change amxhandler.h so that it doesn't return 0 for an unregistered handle, but instead constructs a new handler and returns it. Since the original code would crash anyway in this case, this is not a breaking change to any other scripts.

Thanks for your time. I will report any other compatibility issues I find (which I cannot fix myself).

Zeex commented 6 years ago

There is some important initialization performed during AmxLoad, can't skip this step. I just added a null check to prevent this crash when amx_Exec() is called for an unknown AMX.

Zeex commented 6 years ago

Can you please test and see if this works for your use case?

Zeex commented 6 years ago

I think there are system APIs on both Windows and Linux to enumerate dynamically loaded libraries, you could use that to check which of them are from the plugins directory and call AmxLoad() if needed. But this will not work for plugins outside of that directory.

Edit:

Here it is:

https://github.com/Zeex/samp-plugin-crashdetect/blob/7941665ca10c51fb4682796bc9295498c5efe901/src/os-win32.cpp#L54 https://github.com/Zeex/samp-plugin-crashdetect/blob/7941665ca10c51fb4682796bc9295498c5efe901/src/os-unix.cpp#L82

IS4Code commented 6 years ago

I will try it, thanks a lot. As for calling AmxLoad, I thought I will not resolve to these somewhat hackish approaches, but I think I will add it as a compatibility option for plugins. Thanks for the code!