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

MiniRacer 0.3.x, 0.4.x hangs on exit after fork #170

Closed
tisba opened this issue Jul 24, 2020 · 11 comments
Closed

MiniRacer 0.3.x, 0.4.x hangs on exit after fork #170

tisba opened this issue Jul 24, 2020 · 11 comments

Comments

@tisba
Copy link
Collaborator

tisba commented Jul 24, 2020

Solved. tl;dr:

  • make sure to run latest bundler
  • use mini_racer >= 0.6.2
  • set MiniRacer::Platform.set_flags!(:single_threaded) prior to forking

While updating mini_racer, we noticed a hang/timeout in our CI. I managed to isolate the issue to our usage of the parallel gem. To trigger the behaviour, I have to use MiniRacer first.

I am aware that this might be an issue with parallel not mini_racer, but maybe you can help to point out the right direction. Any help is greatly apprechiated! 💕

This is a minimal script to reproduce the issue. It does not seem to happen under macOS (tested on 10.14.6), but is happening e.g. with docker run -it --rm ruby:2.7.1. In case you are unfamiliar with parallel, Parallel.each([1], in_processes: 1) {} does fork the current process and executes the provided block in parallel (while doing some coordination etc).

# frozen_string_literal: true

# Run with:
#   ruby script.rb
#   -or via docker-
#   docker run -it --rm -v "$(pwd)":/app ruby:2.7.3 ruby /app/mini_racer.rb
#   docker run -it --rm -v "$(pwd)":/app ruby:3.0.1 ruby /app/mini_racer.rb

require "bundler/inline"

gemfile do
  source "https://rubygems.org"
  gem "parallel", "1.20.1"

  # --> broken
  gem "mini_racer", "0.4.0"
  # gem "libv8", "8.4.255.0"
  gem "libv8-node", "15.14.0.1"

  # --> works
  # gem "mini_racer", "0.2.15"
  # gem "libv8", "7.3.492.27.1"
end

puts "parent pid #{Process.pid}"

MiniRacer::Context.new.dispose

Parallel.each([1], in_processes: 1) do |i|
  puts "child pid #{Process.pid}"
end

# we will not get here :(
puts "DONE!"

Some more context: We are using MiniRacer for parts of our core business logic, by evaluating user-provided JavaScripts. The parallel gem is used in a totally different and unrelated spot, but it might happen in the same process (e.g. via Sidekiq).

Currently broken combinations:

  • Ruby 3.0.1, Ruby 2.7.3
  • mini_racer 0.4.0 with libv8-node 15.14.0.1 or libv8 8.4.255.0
  • mini_racer 0.3.1, with libv8 8.4.255.0
@tisba
Copy link
Collaborator Author

tisba commented Jul 28, 2020

The parallel maintainer @grosser suggested that it might be related to an at_exit handler. I'm having a hard time following the native extension of mini_racer but there seems to be some code regarding finalising: https://github.com/rubyjs/mini_racer/blob/v0.3.1/ext/mini_racer_extension/mini_racer_extension.cc#L1639-L1647

What can I do to further help investigating this?

@SamSaffron
Copy link
Collaborator

@cataphract any ideas here, this is due to #2b1ee800 I guess

@tisba
Copy link
Collaborator Author

tisba commented Aug 13, 2020

I'm not sure exactly what part of parallel causes this issue. So far I was not able to reduce the example further down which I think should be possible though.

Is there anything else I can provide to analyse/fix the issue?

@SamSaffron
Copy link
Collaborator

SamSaffron commented Nov 11, 2020

We just committed a patch that will improve things a bit by @cataphract but we are stuck with

#0  0x00007f6e9543334d in pthread_cond_broadcast@@GLIBC_2.3.2 () from /usr/lib/libpthread.so.0
#1  0x00007f6e90ccbb25 in v8::platform::DelayedTaskQueue::Terminate() ()
   from /home/sam/Source/mini_racer/lib/mini_racer_extension.so
#2  0x00007f6e90ccad1d in v8::platform::DefaultWorkerThreadsTaskRunner::Terminate() ()
   from /home/sam/Source/mini_racer/lib/mini_racer_extension.so
#3  0x00007f6e90cc9af6 in v8::platform::DefaultPlatform::~DefaultPlatform() ()
   from /home/sam/Source/mini_racer/lib/mini_racer_extension.so
#4  0x00007f6e90cc9bfe in v8::platform::DefaultPlatform::~DefaultPlatform() ()
   from /home/sam/Source/mini_racer/lib/mini_racer_extension.so

(trivial repro is to run bundle exec ruby test/test_forking.rb)

The only clean resolution here is to wait on v8 to merge https://chromium-review.googlesource.com/c/v8/v8/+/2416501 and get single thread support going.

We could I guess fiddle with exit handlers and force a bypass on v8 exit handlers (which is very yucky and most likely break in spectacular ways)

We could possibly shutdown the platform at fork, but that will break a bunch of other stuff.

I wish there was an easy fix here, but sadly there is none.

@SamSaffron SamSaffron changed the title Breaking change between 0.2.15 and 0.3.x? MiniRacer 0.3.x hangs on exit after fork Nov 11, 2020
@tisba
Copy link
Collaborator Author

tisba commented Nov 24, 2020

Thank you for the update. For the time being it's okay for us to stick with the latest version that works for us.

I'm not that familiar with the chromium review page, but it states: "merged" - sounds promissing 😀

@tisba
Copy link
Collaborator Author

tisba commented Feb 12, 2021

Sorry for a "ping", not wanting to be pushy 🙈 Is there any update to this issue and the single thread support?

@SamSaffron
Copy link
Collaborator

No worries at all, waiting for @lloeki to finish migrating us to node based v8 builds, once that is done we can merge in the mini racer changes.

I am quite excited about this, it will be the first library on the planet to support forking of v8 processes. It is quite monumental, even node does not support this. :)

@tisba
Copy link
Collaborator Author

tisba commented Apr 7, 2021

I just saw that @lloeki merged #186 🥳

I updated the example in the description to use mini_racer master, but unfortunately it does not seem to make any difference. Did I read too much into your last comment? 😀

@lloeki
Copy link
Collaborator

lloeki commented Apr 7, 2021

That merge is a prerequisite to moving forward with fork compatibility (notably because it needed a more recent V8, which we can achieve more easily with libv8-node) but IIUC isn't sufficient in itself.

@tisba
Copy link
Collaborator Author

tisba commented Apr 7, 2021

I see, thanks! btw, I also did some experiments on Linux and arm64 (via Docker on my M1 Mac Mini) and there the problem does not exist. Will do some more investigation in case that's helpful to pin down the issue further.

@tisba
Copy link
Collaborator Author

tisba commented Jan 17, 2022

I'm going to close this issue as with 0.6.2 and MiniRacer::Platform.set_flags!(:single_threaded) this problem is solved for me.

Thanks a lot @lloeki and @SamSaffron 👏 🙇

@tisba tisba closed this as completed Jan 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants