Kill misbehaving client scripts via an interrupt hook#25
Kill misbehaving client scripts via an interrupt hook#25
Conversation
|
Here's how it looks in game: Client.script.dies.mov |
| if os.clock() - lastSteppedStart >= SCRIPT_TIMEOUT then | ||
| error(`Script timeout: exhausted allowed execution time ({SCRIPT_TIMEOUT})`, 2) | ||
|
|
||
| return sandbox.Terminator() |
There was a problem hiding this comment.
This line will probably never run.
Though it does raise a question, should we kill the whole script or only the thread? Killing the whole script and all it's owned threads seems reasonable, however someone could just bypass this by using NLS() to spawn a new script. I actually don't remember if we already do this, but this could be solved by tracking "child" scripts and killing them too.
Another problem with killing the whole script is that it's not normal script behavior, which does raise concerns, however the upsides may be worth it.
There was a problem hiding this comment.
I raised the error as I wanted the user to visually see why the script was stopped and the return sandbox.Terminator() line being there was a mistake. If sandbox.Terminator() kills child scripts, we should use that instead alongside a red output line.
While this is nonstandard, we can always add a toggle in the UI to enable/disable this feature (such as how we did in LSB with repr).
About the child script tracking though - I don't think we currently track child scripts. I do remember implementing a fix for LSB but not for OpenSB. I can test if we do later but that fix would be a separate PR, blocking this one.
ewd3v
left a comment
There was a problem hiding this comment.
Looks good, though take a look at the other comment I made.
Close #24
Also adds @Native to luau_execute (the VM step function) for performance
The
SCRIPT_TIMEOUTis currently 0.1 seconds, which should probably be increased to another value. I chose 0.1, so the user would only experience a slight stutter.https://github.com/rce-incorporated/Fiu/blob/main/Source.lua was used as a reference when adding back interruptHook support.