rubyjs / therubyracer

Embed the V8 Javascript Interpreter into Ruby
1.66k stars 192 forks source link

Extended support for context timeouts to function calls #287

Open dedene opened 10 years ago

dedene commented 10 years ago

Extended the protection against DoS introduced by @SamSaffron in #280 to function calls.

cxt = V8::Context.new timeout: 700
ctx.eval("var dos = function() { while(true){} };")
ctx['dos'].call #= exception after 700ms!
SamSaffron commented 10 years ago

cool, I wonder if we can reused this code as opposed to having it cut-and-pasted in 2 spots?

dedene commented 10 years ago

I agree, however I'm not much of an expert in C.. Any ideas how we could extract the code

VALUE Function::[...]WithTimeout(..., VALUE timeout) {
    pthread_t breaker_thread;
    timeout_data data;
    VALUE rval;
    void *res;

    data.isolate = v8::Isolate::GetCurrent();
    data.timeout = NUM2LONG(timeout);

    pthread_create(&breaker_thread, NULL, rr::breaker, &data);

    rval = [...]

    pthread_cancel(breaker_thread);
    pthread_join(breaker_thread, &res);

    return rval;
  }

into a shared function?

mattconnolly commented 10 years ago

Can one of the admins verify this patch?

SamSaffron commented 10 years ago

I think the fix is correct, however I am also uncertain about how to centralize this code @cowboyd any ideas?

niko commented 7 years ago

Wouldn't that be the solution to #395? Any progress here?

SamSaffron commented 7 years ago

I have been through dealing with timeout bigtime with mini_racer, this gets nasty hairy, especially if you want to kick timeouts when an attached proc is running.

I ended up giving up using an unmanged breaker thread and instead use a pure ruby thread for it which ends up being significantly safer

niko commented 7 years ago

@SamSaffron how do you use a pure rb thread for that? Using Timeout::timeout()?

SamSaffron commented 7 years ago

@niko I use: https://github.com/discourse/mini_racer/blob/master/lib/mini_racer.rb#L216-L236

This works great cause mini racer releases the GIL when it runs JS.