tbodt / v8py

Write Python APIs, then call them from JavaScript using the V8 engine.
GNU Lesser General Public License v3.0
440 stars 28 forks source link

Added "timeout" argument for a JSFunction call #15

Closed desertkun closed 6 years ago

desertkun commented 6 years ago

JSFunction's __call__ method lacks the timeout functionality like eval has.

function freeze() {
    while(true) {};
}

Calling freeze method will hang python for good:

ctx = Context()
ctx.eval( [ code above ] )
ctx.glob.freeze() # will never return

This pull request solves the issue like eval, adding optional timeout argument to **kwargs:

ctx = Context()
ctx.eval( [ code above ] )
try:
    ctx.glob.freeze(timeout=1.0)
except JavaScriptTerminated:
    pass
tbodt commented 6 years ago

I like this interface better:

with v8py.timeout(1.0):
    ctx.glob.freeze()

This leaves the possibility open for keyword arguments to a JavaScript function to do something like creating an object and passing that as an argument.

desertkun commented 6 years ago

Well, current implementation of __call__ has no **kwargs processing whatsoever, because all arguments that go to javascript are passed via *args. Also, it would be really nice to keep api consistent to the eval(..., timeout=1.0) call.

I agree about using with statement, but

  1. I would recommend also to remove timeout argument from eval. That would break compatibility for some users.
  2. timeout has much of sense in context of single javascript call, so using with here is a little bit of overkill
  3. How this api can be implemented in the first place? What if you're in the middle of python code within with and suddenly breaker_thread kicks in and interrupts javascript execution?
tbodt commented 6 years ago

Yes, __call__ currently doesn't use kwargs, but I want to leave the possibility open.

It would be reasonable to remove the timeout argument from eval, but I have no idea if anything is dependent on it. At least, it wouldn't be hard for those users to fix it.

with is arguably overkill, but I can't think of a better way. Plus you might want to run multiple JavaScript things with one timeout. (Maybe.)

The implementation would just be __enter__ starting a thread, __exit__ canceling it, and the thread doing isolate->TerminateExecution() after a timeout. TerminateExecution doesn't do anything if there is no JavaScript currently running.

desertkun commented 6 years ago

Closed as bad API design.