FIX: to keep api consistent ensure_gc_after_idle should be in ms

Also introduce strict param checking for all params passed to context

This avoids typo errors being totally hidden from user

CHANGELOG

@@ -1,6 +1,14 @@
 - 15-05-2020
 
+- 0.2.14
+
+  - FIX: ensure_gc_after_idle should take in milliseconds like the rest of the APIs not seconds
+  - FEATURE: strict params on MiniRacer::Context.new
+
+- 15-05-2020
+
 - 0.2.13
+
    - FIX: edge case around ensure_gc_after_idle possibly firing when context is not idle
 
 - 15-05-2020

README.md

@@ -239,7 +239,7 @@ This can come in handy to force V8 GC runs for example in between requests if yo
 
 Note that this method maps directly to [`v8::Isolate::IdleNotification`](http://bespin.cz/~ondras/html/classv8_1_1Isolate.html#aea16cbb2e351de9a3ae7be2b7cb48297), and that in particular its return value is the same (true if there is no further garbage to collect, false otherwise) and the same caveats apply, in particular that `there is no guarantee that the [call will return] within the time limit.`
 
-Additionally you may automate this process on a context by defining it with `MiniRacer::Content.new(ensure_gc_after_idle: 1)`. Using this will ensure V8 will run a full GC using `context.isolate.low_memory_notification` 1 second after the last eval on the context. Low memory notification is both slower and more aggressive than an idle_notification and will ensure long living isolates use minimal amounts of memory.
+Additionally you may automate this process on a context by defining it with `MiniRacer::Content.new(ensure_gc_after_idle: 1000)`. Using this will ensure V8 will run a full GC using `context.isolate.low_memory_notification` 1 second after the last eval on the context. Low memory notification is both slower and more aggressive than an idle_notification and will ensure long living isolates use minimal amounts of memory.
 
 ### V8 Runtime flags
 

lib/mini_racer.rb

@@ -130,23 +130,22 @@ def initialize(name, callback, parent)
       end
     end
 
-    def initialize(options = nil)
+    def initialize(max_memory: nil, timeout: nil, isolate: nil, ensure_gc_after_idle: nil, snapshot: nil)
       options ||= {}
 
-      check_init_options!(options)
+      check_init_options!(isolate: isolate, snapshot: snapshot, max_memory: max_memory, ensure_gc_after_idle: ensure_gc_after_idle, timeout: timeout)
 
       @functions = {}
       @timeout = nil
       @max_memory = nil
       @current_exception = nil
-      @timeout = options[:timeout]
-      if options[:max_memory].is_a?(Numeric) && options[:max_memory] > 0
-        @max_memory = options[:max_memory]
-      end
+      @timeout = timeout
+      @max_memory = max_memory
+
       # false signals it should be fetched if requested
-      @isolate = options[:isolate] || false
+      @isolate = isolate || false
 
-      @ensure_gc_after_idle = options[:ensure_gc_after_idle]
+      @ensure_gc_after_idle = ensure_gc_after_idle
 
       if @ensure_gc_after_idle
         @last_eval = nil
@@ -162,7 +161,7 @@ def initialize(options = nil)
       @eval_thread = nil
 
       # defined in the C class
-      init_unsafe(options[:isolate], options[:snapshot])
+      init_unsafe(isolate, snapshot)
     end
 
     def isolate
@@ -290,6 +289,7 @@ def ensure_gc_thread
       @ensure_gc_mutex.synchronize do
         @ensure_gc_thread = nil if !@ensure_gc_thread&.alive?
         @ensure_gc_thread ||= Thread.new do
+          ensure_gc_after_idle_seconds = @ensure_gc_after_idle / 1000.0
           done = false
           while !done
             now = Process.clock_gettime(Process::CLOCK_MONOTONIC)
@@ -299,7 +299,7 @@ def ensure_gc_thread
               break
             end
 
-            if !@eval_thread && @ensure_gc_after_idle < now - @last_eval
+            if !@eval_thread && ensure_gc_after_idle_seconds < now - @last_eval
               @ensure_gc_mutex.synchronize do
                 isolate_mutex.synchronize do
                   if !@eval_thread
@@ -310,7 +310,7 @@ def ensure_gc_thread
                 end
               end
             end
-            sleep @ensure_gc_after_idle if !done
+            sleep ensure_gc_after_idle_seconds if !done
           end
         end
       end
@@ -369,15 +369,29 @@ def timeout(&blk)
       rp.close if rp
     end
 
-    def check_init_options!(options)
-      assert_option_is_nil_or_a('isolate', options[:isolate], Isolate)
-      assert_option_is_nil_or_a('snapshot', options[:snapshot], Snapshot)
+    def check_init_options!(isolate:, snapshot:, max_memory:, ensure_gc_after_idle:, timeout:)
+      assert_option_is_nil_or_a('isolate', isolate, Isolate)
+      assert_option_is_nil_or_a('snapshot', snapshot, Snapshot)
 
-      if options[:isolate] && options[:snapshot]
+      assert_numeric_or_nil('max_memory', max_memory, min_value: 10_000)
+      assert_numeric_or_nil('ensure_gc_after_idle', ensure_gc_after_idle, min_value: 1)
+      assert_numeric_or_nil('timeout', timeout, min_value: 1)
+
+      if isolate && snapshot
         raise ArgumentError, 'can only pass one of isolate and snapshot options'
       end
     end
 
+    def assert_numeric_or_nil(option_name, object, min_value:)
+      if object.is_a?(Numeric) && object < min_value
+        raise ArgumentError, "#{option_name} must be larger than #{min_value}"
+      end
+
+      if !object.nil? && !object.is_a?(Numeric)
+        raise ArgumentError, "#{option_name} must be a number, passed a #{object.inspect}"
+      end
+    end
+
     def assert_option_is_nil_or_a(option_name, object, klass)
       unless object.nil? || object.is_a?(klass)
         raise ArgumentError, "#{option_name} must be a #{klass} object, passed a #{object.inspect}"

lib/mini_racer/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module MiniRacer
-  VERSION = "0.2.13"
+  VERSION = "0.2.14"
 end

test/mini_racer_test.rb

@@ -326,8 +326,9 @@ def test_max_memory_for_call
   end
 
   def test_negative_max_memory
-    context = MiniRacer::Context.new(max_memory: -200_000_000)
-    assert_nil(context.instance_variable_get(:@max_memory))
+    assert_raises(ArgumentError) do
+      MiniRacer::Context.new(max_memory: -200_000_000)
+    end
   end
 
   module Echo
@@ -722,8 +723,14 @@ def test_releasing_memory
     assert((end_heap - start_heap).abs < 1000, "expecting most of the 1_000_000 long string to be freed")
   end
 
+  def test_bad_params
+    assert_raises do
+      MiniRacer::Context.new(random: :thing)
+    end
+  end
+
   def test_ensure_gc
-    context = MiniRacer::Context.new(ensure_gc_after_idle: 0.001)
+    context = MiniRacer::Context.new(ensure_gc_after_idle: 1)
     context.isolate.low_memory_notification
 
     start_heap = context.heap_stats[:used_heap_size]