Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interrupt a script/eval #7

Closed
asdfgasdfsafgsdfa opened this issue Feb 29, 2016 · 6 comments
Closed

Interrupt a script/eval #7

asdfgasdfsafgsdfa opened this issue Feb 29, 2016 · 6 comments

Comments

@asdfgasdfsafgsdfa
Copy link

Hi, im using jurassic to let users run scripts, but i need to add a timeout for them so they can't mess up my server with "while(true) ;"

Is there any callback or something to do that?
Or do I have to go the ugly route of Thread.Abort() ?? That'd be pretty sad :(

@paulbartrum
Copy link
Owner

I've been asked this before. The problem is that there is only one reliable way to execute untrusted code, and that is to run it inside it's own process. A timeout will not save you; for example, the script could allocate arbitrary amounts of memory, or it could cause a stack overflow exception by recursively calling a function a million times. (The stack overflow exception, in particular, will inevitably result in your app crashing).

I do actually need to do something similar to run the unit test suite reliably. If I can make it generic enough, I'll consider pulling this code out into a couple of helper classes.

@pvginkel
Copy link

I'm wondering: what's the problem with using Thread.Abort(). I know it's not ideal, but there are no real alternatives when compiling to MSIL. In e.g. Jint since it's an interpreter, it's easy to do something after a/any statement. With MSIL the only way to do that is through Thread.Abort(). How that method works is that there are certain points where the .NET runtime checks whether there is a Thread.Abort() pending and inserts an exception there. This kind of looks like exactly what you'd want to happen. Any implementation in Jurassic itself to accomplish this would be implemented in a similar method, i.e. have a bool somewhere that it checks at e.g. back branches and function call's/returns. In the end the effect is roughly the same.

I wouldn't shy back from using Thread.Abort() for this.

@asdfgasdfsafgsdfa
Copy link
Author

@pvginkel

I want to let js call stuff in my .net code.
That code takes locks, Thread.Abort will not release them again.
There are a few other cases when im writing to my logfile (some messages are up to 3k characters).

The logfile stuff i could find other non-ideal solutions for. But releasing locks? Not that I'm aware of.

@paulbartrum
Copy link
Owner

Thread.Abort() is fine if you just want to implement a time-out. It's not fine if you are trying to safely execute untrusted code. My belief is that people who ask for the former generally actually need the latter.

@pvginkel
Copy link

Locks should not be a problem if you implement them correctly. Thread.Abort will let finalizers run. The documentation specifically states this. This means that if you use finally, using or lock, this will work correctly.

About untrusted code. Generally C# is supposed to be written exception safe. This means that you're supposed to always clean up in a finally (or using or lock). If the untrusted code is not, you have a bigger problem. As for pinvoke's ([DllImport]), the documentation for Thread.Abort specifically states that it won't abort while inside a pinvoke. How Thread.Abort works is that there are certain places in the jitted code that .NET checks for some work. These are points that e.g. the garbage collector runs. Thread.Abort will insert an exception in those points.

I know that Thread.Abort is being frowned upon, but there are valid use cases for it. And, don't forget, ASP.NET uses it a lot to recycle AppDomain's.

@asdfgasdfsafgsdfa
Copy link
Author

That is very interesting. Thank you for that writeup.
I was not aware of how exactly Thread.Abort worked, I just knew that it was frowned upon (which is pretty dumb when you frown upon stuff just because you dont fully understand it).

So thanks again for clearing that up.

I will test Thread.Abort in a few usage scenarios then and see if it really works well for my case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants