Bug 3665 - Server crashes when a message is sent during register_message forward
Server crashes when a message is sent during register_message forward
Status: RESOLVED WONTFIX
Product: AMX Mod X
Classification: Unclassified
Component: Core
trunk
PC All
: P3 normal
Assigned To: amxmodx-bugs@alliedmods.net
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-02-24 20:23 PST by WiLS (MeRcyLeZZ)
Modified: 2013-07-03 12:37 PDT (History)
3 users (show)

See Also:


Attachments
Fakemeta hooking (re-entrancy prevented) (2.66 KB, text/plain)
2009-04-12 00:08 PDT, WiLS (MeRcyLeZZ)
no flags Details

Description WiLS (MeRcyLeZZ) 2009-02-24 20:23:39 PST
Overview:

When an engine-generated message or emessage is sent during the callback (forward) of a message hooked with register_message, it will either crash the server or cause clients to be kicked out with a message parsing error (svc_bad).

Definitions:

A. Engine-generated messages
Sent by the game as a result of executing some code, e.g. give_item(index, "weapon_awp") will automatically send a "WeapPickup" message.

B. Emessages
The ones that can be received by other metamod plugins, e.g. emessage_begin(), ewrite_byte(), etc.

Steps to Reproduce:

1. Register any message with register_message.
2. Add one of the aforementioned message-triggering codes to its forward.
3. Test plugin and take note of the results.

Actual Results:
The server crashes with a message related error, or the client gets kicked with a message parsing error (svc_bad and such).

Expected Results:
The messages should be sent normally without the server crashing or clients getting kicked.

Additional Information:
Details and some thorough testing results can be found at the following thread:
http://forums.alliedmods.net/showthread.php?t=85161&page=2
Comment 1 David Anderson [:dvander] 2009-02-24 21:12:15 PST
Expected results are actual results.  You shouldn't be doing this.  We can document it or make it throw an RTE instead if you'd like.
Comment 2 WiLS (MeRcyLeZZ) 2009-03-23 10:18:57 PDT
An RTE would be better than a server crash, yeah.

Actually, I can always get around this by setting a 0.1 sec task to send the messages, but it's a bit of hacky/unefficient solution.

It would be awesome if AMXX could handle the messages that would result in this re-entrancy bug and delay their sending/parsing until the currently parsed message is dealt with. I understand this might be asking a lot though, maybe this should be turned into a feature request instead.
Comment 3 David Anderson [:dvander] 2009-03-23 11:04:51 PDT
The point is that there's no bug here.  The actual results are what I would expect.  The server is not re-entrant, you can only be in one message at a time.  There's only so many pathways where we can prevent misuse of the engine, while we try to catch as many as possible there's diminishing returns - documentation pays off more than extensive "security" checks.

Creating timers is almost never the right solution.  They introduce non-determinism into your code because you don't actually know when the timer will execute (though, that may not be a problem - but if it is it's usually subtle).  You should just use something other than register_message, like register_event which is a post hook.
Comment 4 WiLS (MeRcyLeZZ) 2009-04-12 00:06:40 PDT
> Expected results are actual results.
Well, I managed to hook a message just by using FakeMeta in a plugin and was successful at getting the engine-generated messages sent during its "forward" without the server crashing. The implementation is pretty much the same as in the current AMXX's core, so I believe there is a workaround after all. I've attached the code in case you're interested.

> Creating timers is almost never the right solution.
> You should just use something other than register_message, like register_event which is a post hook.
Yeah, only there seems to be something wrong with register_event's forwards too (as you may have seen in bug 3664). In the end, the safest methods turn out to be either using timers or having your plugin build up some sort of message stack to be processed on the following server frame forward.
Comment 5 WiLS (MeRcyLeZZ) 2009-04-12 00:08:14 PDT
Created attachment 1530 [details]
Fakemeta hooking (re-entrancy prevented)

Note You need to log in before you can comment on or make changes to this bug.