Another way to limit javascript execution times

[o.go...@gmail.com]

Hi,

I have seen a few methods on this list to limit rhino script execution
time; to catch infinite loops in user scripts and such.

I was struggling with this too, and have come up with the following
implementation. It is a bit 'dangerous' since it relies on Thread.stop
and friends, which are deprecated for good reasons, so consider
yourself warned. It is however a method with a lot less overhead
compared to the alternatives. It is useful in a server context where
you wish to execute lots of compiled (and cached) javascript.

Basically, before executing any script, schedule a callback in the
future on another thread (using java.util.Timer for instance). When
the timeout callback runs, and the script has not finished executing,
do the following (adapted simplified example code):

t.suspend(); // suspend the thread running the script
try {
StackTraceElement[] sts = t.getStackTrace();
if (sts.length > 0 && sts[0].getClassName().startsWith
("org.mozilla.javascript.")) {
Throwable exception = new Error("User script is executing
too long");
exception.setStackTrace(t.getStackTrace());
t.stop(exception);
return;
}
} finally {
t.resume();
}

Since we don't blindly stop the other thread, but only if its top
frame is executing something inside the rhino jar. We can safely
handle the exception, and discard anything associated with this
thread. (Including the rhino context and global objects.) This does
imply you kill the script or session or whatever makes sense in your
environment, with some sort of fatal error.

As long as you are prepared (everywhere in your code) to handle any
kind of error from the rhino library, you should be ok; so never call
into rhino from inside synchronized blocks or such.

Likely you would want to retry killing the script thread a few times,
before going for more drastic measurements like a System.exit or just
leave the runaway script running.

The reason you should consider anything associated with the script
corrupt, is because of: http://java.sun.com/javase/6/docs/technotes/guides/concurrency/threadPrimitiveDeprecation.html

But because we are prepared to handle random rhino errors, and rhino
does not share data beyond the current context and scope, all
corruption is basically local to whatever was associated with the
script.

feedback is appreciated

regards,
-Onne

[Norris Boyd]

> corrupt, is because of:http://java.sun.com/javase/6/docs/technotes/guides/concurrency/thread...

>
> But because we are prepared to handle random rhino errors, and rhino
> does not share data beyond the current context and scope, all
> corruption is basically local to whatever was associated with the
> script.
>
> feedback is appreciated
>
> regards,
> -Onne

Just curious, why did the observeInstructionCount approach not work
for you?

Thanks,
Norris

[o.go...@gmail.com]

it won't work on
while(true);

And is about 3 times slower on my (primitive) benchmarks, while we
need all the speed we can get.

Also this adds some protection against java plugins, which our system
allows, but only in case they are long/slow running, not when looping.

(The user scripts (or plugins) are friendly, so I don't need to worry
about malicious scripts, just bugs.)

-Onne

> Thanks,
> Norris

[Norris Boyd]

I believe there was a bug at one time that kept Rhino from catching
this case (although a few searches in bugzilla aren't turning up
anything). At any rate, the observeInstructionCount approach will work
on this case now.

>
> And is about 3 times slower on my (primitive) benchmarks, while we
> need all the speed we can get.

I'm a little surprised it's this expensive, but I haven't profiled it
recently to know differently.

>
> Also this adds some protection against java plugins, which our system
> allows, but only in case they are long/slow running, not when looping.

Yes, you're right. observeInstructionCount approach doesn't defend
against this.