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

Rewrite timeout tracking to be in cpu-time instead of wall-time #9

Merged
merged 8 commits into from
Oct 8, 2024
Merged
16 changes: 16 additions & 0 deletions ext/mini_racer_extension/mini_racer_extension.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <mutex>
#include <atomic>
#include <math.h>
#include <cassert>

using namespace v8;

Expand Down Expand Up @@ -98,6 +99,8 @@ typedef struct {
Persistent<Context>* context;
Global<Object> ruby_error_prototype;
Global<Function> capture_stack_trace;
std::atomic<unsigned long> cpu_nanos;
struct timespec cpu_time_reference;
} ContextInfo;

typedef struct {
Expand Down Expand Up @@ -505,6 +508,10 @@ nogvl_context_eval(void* arg) {
StackCounter::SetMax(isolate, eval_params->marshal_stackdepth);
}

int time_status = clock_gettime(CLOCK_THREAD_CPUTIME_ID, &eval_params->context_info->cpu_time_reference);
assert(time_status == 0); // can only hit on EOVERFLOW (which shouldn't happen) and EINVAL (kernel too old; don't care right now)
eval_params->context_info->cpu_nanos.store(0L, std::memory_order_relaxed);

maybe_value = parsed_script.ToLocalChecked()->Run(context);
}

Expand Down Expand Up @@ -1691,6 +1698,14 @@ rb_heap_snapshot(VALUE self, VALUE file) {
return Qtrue;
}

static VALUE
rb_context_current_thread_clock(VALUE self) {
clockid_t thread_clock;
int ok = pthread_getcpuclockid(pthread_self(), &thread_clock);
seanmakesgames marked this conversation as resolved.
Show resolved Hide resolved
assert(ok == 0 && "couldn't grab current thread's cpu clock"); // TODO: rb_raise an error instead
return INT2NUM(thread_clock);
}

static VALUE
rb_context_stop(VALUE self) {

Expand Down Expand Up @@ -1923,6 +1938,7 @@ extern "C" {
rb_define_private_method(rb_cContext, "call_unsafe", (VALUE(*)(...))&rb_context_call_unsafe, -1);
rb_define_private_method(rb_cContext, "isolate_mutex", (VALUE(*)(...))&rb_context_isolate_mutex, 0);
rb_define_private_method(rb_cContext, "init_unsafe",(VALUE(*)(...))&rb_context_init_unsafe, 2);
rb_define_private_method(rb_cContext, "current_thread_clock", (VALUE(*)(...))&rb_context_current_thread_clock, 0);

rb_define_alloc_func(rb_cContext, allocate);
rb_define_alloc_func(rb_cSnapshot, allocate_snapshot);
Expand Down
37 changes: 29 additions & 8 deletions lib/mini_racer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,16 +143,18 @@ def initialize(name, callback, parent)
end
end

def initialize(max_memory: nil, timeout: nil, isolate: nil, ensure_gc_after_idle: nil, snapshot: nil, marshal_stack_depth: nil)
def initialize(max_memory: nil, timeout: nil, cputime_limit: nil, isolate: nil, ensure_gc_after_idle: nil, snapshot: nil, marshal_stack_depth: nil)
options ||= {}

check_init_options!(isolate: isolate, snapshot: snapshot, max_memory: max_memory, marshal_stack_depth: marshal_stack_depth, ensure_gc_after_idle: ensure_gc_after_idle, timeout: timeout)
check_init_options!(isolate: isolate, snapshot: snapshot, max_memory: max_memory, marshal_stack_depth: marshal_stack_depth, ensure_gc_after_idle: ensure_gc_after_idle, timeout: timeout, cputime_limit: cputime_limit)

@functions = {}
@timeout = nil
@cputime_limit = nil
seanmakesgames marked this conversation as resolved.
Show resolved Hide resolved
seanmakesgames marked this conversation as resolved.
Show resolved Hide resolved
@max_memory = nil
@current_exception = nil
@timeout = timeout
@cputime_limit = cputime_limit
@max_memory = max_memory
seanmakesgames marked this conversation as resolved.
Show resolved Hide resolved
@marshal_stack_depth = marshal_stack_depth

Expand Down Expand Up @@ -340,19 +342,37 @@ def stop_attached
end

def timeout(&blk)
return blk.call unless @timeout
return blk.call unless @timeout or @cputime_limit

mutex = Mutex.new
done = false

rp,wp = IO.pipe

start_monotonic_time = Process.clock_gettime Process::CLOCK_MONOTONIC, :millisecond
thread_clock = nil
if @cputime_limit then
# this call might fail, so condition on actually needing it
thread_clock = current_thread_clock
start_cpu_time = Process.clock_gettime thread_clock, :millisecond
seanmakesgames marked this conversation as resolved.
Show resolved Hide resolved
end

t = Thread.new do
begin
result = IO.select([rp],[],[],(@timeout/1000.0))
if !result
mutex.synchronize do
stop unless done
while true do
result = IO.select([rp],[],[],0.05) # seconds # TODO: Adjust this interval depending on the remaining wall/cpu clock; required for tests
break if result # operation completed
current_monotonic_time = Process.clock_gettime Process::CLOCK_MONOTONIC, :millisecond
if thread_clock then
current_cpu_time = Process.clock_gettime thread_clock, :millisecond
end
if
(@timeout and current_monotonic_time - start_monotonic_time >= @timeout) or
(@cputime_limit and current_cpu_time - start_cpu_time >= @cputime_limit)
then
mutex.synchronize do
stop unless done
end
end
end
rescue => e
Expand Down Expand Up @@ -383,14 +403,15 @@ def timeout(&blk)
rp.close if rp
end

def check_init_options!(isolate:, snapshot:, max_memory:, marshal_stack_depth:, ensure_gc_after_idle:, timeout:)
def check_init_options!(isolate:, snapshot:, max_memory:, marshal_stack_depth:, ensure_gc_after_idle:, timeout:, cputime_limit:)
assert_option_is_nil_or_a('isolate', isolate, Isolate)
assert_option_is_nil_or_a('snapshot', snapshot, Snapshot)

assert_numeric_or_nil('max_memory', max_memory, min_value: 10_000, max_value: 2**32-1)
assert_numeric_or_nil('marshal_stack_depth', marshal_stack_depth, min_value: 1, max_value: MARSHAL_STACKDEPTH_MAX_VALUE)
assert_numeric_or_nil('ensure_gc_after_idle', ensure_gc_after_idle, min_value: 1)
assert_numeric_or_nil('timeout', timeout, min_value: 1)
assert_numeric_or_nil('cputime_limit', cputime_limit, min_value: 1)

if isolate && snapshot
raise ArgumentError, 'can only pass one of isolate and snapshot options'
Expand Down
28 changes: 24 additions & 4 deletions test/mini_racer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,21 +102,40 @@ def test_it_can_stop
end

def test_it_can_timeout_during_serialization
context = MiniRacer::Context.new(timeout: 500)
context = MiniRacer::Context.new(timeout: 20)

assert_raises(MiniRacer::ScriptTerminatedError) do
context.eval 'var a = {get a(){ while(true); }}; a'
end
end

def test_it_can_automatically_time_out_context
# 2 millisecs is a very short timeout but we don't want test running forever
def test_it_can_time_out_js_eval
context = MiniRacer::Context.new(timeout: 2)
assert_raises do
context.eval('while(true){}')
end
end

def test_it_can_hit_cputime_limit_during_serialization
context = MiniRacer::Context.new(timeout: 30,cputime_limit: 20)

Timeout::timeout(0.250) do
assert_raises(MiniRacer::ScriptTerminatedError) do
context.eval 'var a = {get a(){ while(true); }}; a'
end
end
end

def test_it_can_limit_cputime
context = MiniRacer::Context.new(timeout: 20, cputime_limit: 2)
# our test should raise after the cputime_limit, but before our timeout
Timeout::timeout(0.100) do
assert_raises do
context.eval('while(true){}')
end
end
end

def test_returns_javascript_function
context = MiniRacer::Context.new
assert_same MiniRacer::JavaScriptFunction, context.eval("var a = function(){}; a").class
Expand Down Expand Up @@ -640,7 +659,8 @@ def test_function_rval
end

def test_timeout_in_ruby_land
context = MiniRacer::Context.new(timeout: 50)
# Our cpu limit should not be hit, because we are sleeping
context = MiniRacer::Context.new(timeout: 50, cputime_limit:2)
context.attach('sleep', proc{ sleep 0.1 })
assert_raises(MiniRacer::ScriptTerminatedError) do
context.eval('sleep(); "hi";')
Expand Down
Loading