rails / execjs

Run JavaScript code from Ruby
MIT License
540 stars 282 forks source link

Possible Race Condition #19

Open Jared-Prime opened 9 years ago

Jared-Prime commented 9 years ago

I'm looking at an error thrown in a Rails application which uses v2.0.2 of execjs. I think it may be due to a potential race condition in this method, but I want to

  1. validate my understanding of the error thrown
  2. check, if my understanding is correct and this has been repaired in a later version of the gem, whether a simple upgrade may resolve the errors

For completeness, the method body (which no longer exists in this manner on most recent version) is

def exec_runtime(filename)
  output = sh("#{shell_escape(*(binary.split(' ') << filename))} 2>&1")
  if $?.success?
    output
  else
    raise RuntimeError, output
  end
end

My understanding so far is that, if the most recent exit code is non-zero, the runtime will raise RuntimeError with the shell output even if the shell script executed successfully. The most recent exit code would not have been from the shell script supplying the output -- i.e. a race condition in examining $?

Thank you for your help.

DrMavenRebe commented 9 years ago

:+1:

boazsegev commented 7 years ago

The child process last status seems to be thread specific, although I'm not sure how this works with fibers, the code should be thread-safe as far as actual threads go...

However, it should be noted that the lasts status is cleared between commands, so if a command returns before the child process completed it's termination (which could happen due to pipes as well as signals), the $? might be nil when tested... making if $?.success? unsafe (use if $? && $?.success?)

However, this hardly resolves the issue, since the assumption is that the child process had terminated and that the Ruby application doesn't "reap" child processes elsewhere in the code.