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

Invalid date causes V8 fatal error #316

Closed
bnoordhuis opened this issue Sep 20, 2024 · 2 comments · Fixed by #317
Closed

Invalid date causes V8 fatal error #316

bnoordhuis opened this issue Sep 20, 2024 · 2 comments · Fixed by #317
Labels
bug/crash Bugs specific to crashes, segfaults, etc. Gem can be installed though.

Comments

@bnoordhuis
Copy link
Collaborator

require "mini_racer"
MiniRacer::Context.new.eval("new Date(0xFFFFFFFF, 0xFFFFFFFF)")
$ bundle exec ruby t.rb
/home/bnoordhuis/src/mini_racer/lib/mini_racer.rb:237:in `eval_unsafe': out of Time range (RangeError)
        from /home/bnoordhuis/src/mini_racer/lib/mini_racer.rb:237:in `block (2 levels) in eval'
        from /home/bnoordhuis/src/mini_racer/lib/mini_racer.rb:358:in `timeout'
        from /home/bnoordhuis/src/mini_racer/lib/mini_racer.rb:236:in `block in eval'
        from /home/bnoordhuis/src/mini_racer/lib/mini_racer.rb:234:in `synchronize'
        from /home/bnoordhuis/src/mini_racer/lib/mini_racer.rb:234:in `eval'
        from t.rb:2:in `<main>'

#
# Fatal error in v8::Isolate::Dispose()
# Disposing the isolate that is entered by a thread
#

Trace/breakpoint trap (core dumped)

"out of Time range" is throw by ruby's rb_time_new() function.

@bnoordhuis bnoordhuis added the bug/crash Bugs specific to crashes, segfaults, etc. Gem can be installed though. label Sep 20, 2024
@bnoordhuis
Copy link
Collaborator Author

new Date(NaN) has the same isuse. I think the calculation here is broken:

double ts = Local<Date>::Cast(value)->ValueOf();
double secs = ts/1000;
long nanos = round((secs - floor(secs)) * 1000000);

@bnoordhuis
Copy link
Collaborator Author

I think I've figured it out:

  1. convert_v8_to_ruby calls rb_time_new calls rb_raise
  2. rb_raise longjmps up the stack
  3. IsolateScope/HandleScope/etc. destructors don't run
  4. V8 is left in an undefined state

rb_time_new is just one instance of this bug category. I saw the same Disposing the isolate that is entered by a thread error after a JSON::ParserError, because mini_racer tries to json-ify the eval result.

bnoordhuis added a commit to bnoordhuis/mini_racer that referenced this issue Sep 21, 2024
mini_racer converts JS objects to Ruby objects. Ruby's C API functions
call rb_raise() on invalid inputs, and rb_raise() in turn is a glorified
longjmp() that jumps up the stack.

Longjmp and C++ destructors do not play well together. It left V8 in an
undefined state, resulting in random "Disposing the isolate that is
entered by a thread" fatal errors.

Fixes: rubyjs#316
bnoordhuis added a commit to bnoordhuis/mini_racer that referenced this issue Sep 21, 2024
mini_racer converts JS objects to Ruby objects. Ruby's C API functions
call rb_raise() on invalid inputs, and rb_raise() in turn is a glorified
longjmp() that jumps up the stack.

Longjmp and C++ destructors do not play well together. It left V8 in an
undefined state, resulting in random "Disposing the isolate that is
entered by a thread" fatal errors.

Fixes: rubyjs#316
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/crash Bugs specific to crashes, segfaults, etc. Gem can be installed though.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant