Update to libv8-node 20.x #284
Conversation
| context = MiniRacer::Context.new(timeout: 5) | ||
| context.eval("let a='testing';") | ||
| # Failure: expecting the isolate to have values for all the vals | ||
| # {:total_physical_size=>835584, :total_heap_size_executable=>0, :total_heap_size=>1392640, :used_heap_size=>345736, :heap_size_limit=>1518338048} |
There was a problem hiding this comment.
total_heap_size_executable seems to always be 0 (at least as tested on arm64-darwin)
|
|
||
| assert(stats.values.all?{|v| v > 0}, "expecting the isolate to have values for all the vals") | ||
| end | ||
| # File.open('testlog', 'wb') { |f| f << stats.inspect } |
There was a problem hiding this comment.
Writing things to a file to debug
test/mini_racer_test.rb
Outdated
| # cuts after c + cuts before d + never e => likely to be Isolate#pump_message_loop | ||
| # def test_webassembly | ||
| # skip "TruffleRuby does not enable WebAssembly by default" if RUBY_ENGINE == "truffleruby" | ||
| # log = File.open('testlog2', 'wb') |
There was a problem hiding this comment.
Writing things to a file to debug
test/mini_racer_test.rb
Outdated
| # Fatal error in HandleScope::HandleScope | ||
| # Entering the V8 API without proper locking in place | ||
| # log has been seen as (in order): | ||
| # abcdcdcdcdcdcdcdcdcdcdcdcdc | ||
| # abcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdc | ||
| # abcdcdcdcdcdcdcdcdcdcdcdcdc | ||
| # abcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdc | ||
| # abcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdc | ||
| # cuts after c + cuts before d + never e => likely to be Isolate#pump_message_loop |
There was a problem hiding this comment.
This one seems to happen randomly after a few loops. Exact cause is yet unknown. Could be some timing, could be message loop doing no work but had some before, could be message loop having some work and none before, could be any other unknown thing really since I did not dig.
There was a problem hiding this comment.
From: #283 (comment) this was fixed by @Fayti1703 in Fayti1703@1e43a49, which I've cherry-picked.
| # segfault | ||
| # CFUNC :init_with_snapshot | ||
| # METHOD /Users/loic.nageleisen/Source/github.com/rubyjs/libv8-node/test/mini_racer/lib/mini_racer.rb:81 (Isolate#initialize) |
There was a problem hiding this comment.
Initialising the isolate with a snapshot burns into flames. Note that it apparently doesn't reach warmup! so probably either unrelated to the above or indirectly related but for yet unknown, deeper reasons.
def initialize(snapshot = nil)
unless snapshot.nil? || snapshot.is_a?(Snapshot)
raise ArgumentError, "snapshot must be a Snapshot object, passed a #{snapshot.inspect}"
end
# defined in the C class
init_with_snapshot(snapshot)
end
static VALUE rb_isolate_init_with_snapshot(VALUE self, VALUE snapshot) {
IsolateInfo* isolate_info;
TypedData_Get_Struct(self, IsolateInfo, &isolate_type, isolate_info);
init_v8();
SnapshotInfo* snapshot_info = nullptr;
if (!NIL_P(snapshot)) {
TypedData_Get_Struct(snapshot, SnapshotInfo, &snapshot_type, snapshot_info);
}
isolate_info->init(snapshot_info);
isolate_info->hold();
return Qnil;
}
| # segfault | ||
| # CFUNC :warmup_unsafe! | ||
| # METHOD /Users/loic.nageleisen/Source/github.com/rubyjs/libv8-node/test/mini_racer/lib/mini_racer.rb:452 (def warmup!) |
There was a problem hiding this comment.
warmup_unsafe! indeed! Either we don't do something that we should to make it safe, or we pass something that was okay before and is not now.
def warmup!(src)
# we have to do something here
# we are bloating memory a bit but it is more correct
# than hitting an exception when attempty to compile invalid source
begin
ctx = MiniRacer::Context.new
ctx.eval(@source)
ctx.eval(src)
rescue MiniRacer::RuntimeError => e
raise MiniRacer::SnapshotError.new, e.message, e.backtrace
end
warmup_unsafe!(src)
end
static VALUE rb_snapshot_warmup_unsafe(VALUE self, VALUE str) {
SnapshotInfo* snapshot_info;
TypedData_Get_Struct(self, SnapshotInfo, &snapshot_type, snapshot_info);
Check_Type(str, T_STRING);
init_v8();
StartupData cold_startup_data = {snapshot_info->data, snapshot_info->raw_size};
StartupData warm_startup_data = warm_up_snapshot_data_blob(cold_startup_data, RSTRING_PTR(str));
if (warm_startup_data.data == NULL && warm_startup_data.raw_size == 0) {
rb_raise(rb_eSnapshotError, "Could not warm up snapshot, most likely the source is incorrect");
} else {
delete[] snapshot_info->data;
snapshot_info->data = warm_startup_data.data;
snapshot_info->raw_size = warm_startup_data.raw_size;
}
return self;
}
|
Heh, just noticed |
|
Echoing @Fayti1703 notes here coz I'm tired of clicking around :D
Might also be that we do (or fail to do) something and we end up corrupting memory / pointer / whatever. Hence why |
Not sure when this became required. Docs do say > The caller has to make sure that this is called from the right thread. but make no mention of required locking.
|
Merging this as is as per the plan. |
Depends on #283: uses
libv8-node-19as a base, which should be merged first.