IS4Code / YSF

YSF Server Functions
Other
98 stars 34 forks source link

Thread-safe callbacks coming from query #85

Open IS4Code opened 6 years ago

IS4Code commented 6 years ago

Since OnRemoteRCONPacket and OnServerQueryInfo are called from the query thread and not the main thread, they could lead to parallel execution of code in a single AMX machine. The AMX implementation cannot handle this, and when this happens, it could cause any sort of issues from data races, stack corruption to potentially leaking heap contents.

An apparent solution would be to schedule these callbacks on server tick, but that poses a security and performance risk – potential DoS attempts would slow down the main thread which would need to execute the callback handlers, and having to wait for the main thread to complete the query would slow down the query mechanism. Despite data races, it is good the query mechanism is kept separate, but the issue arises when communication with the AMX must be performed.

A quick fix is to ensure that these callbacks handlers are separated and isolated. Restricting them to a separate filterscript (that handles only these callbacks) should be sufficient, and the handlers themselves should only call functions which have zero side effects. This ensures that only one thread can enter the AMX at a time, and prevents data races in other code. When calling native functions that use some state, be prepared for indeterministic changes in their results.

I am not sure how to properly address this issue. On one hand, keeping the AMX consistent and stable should be a high priority, but on the other hand, these callbacks can be used in a safe way and must not impair the performance of the server.

I shall start with limiting the callbacks to filterscripts only. Unless you are using some weird modular system with a bare gamemode, the code in the gamemode contains the most critical code which must be protected. However, I don't know where to go with this next, so I leave this topic here for further input from you.

ghost commented 6 years ago

OMG xD

Now you reveled a secret why random crashes happend in those callbacks. Thanks :D