rubyjs / therubyracer

Embed the V8 Javascript Interpreter into Ruby
1.66k stars 192 forks source link

Support IgnoreOutOfMemoryException #223

Closed Ecco closed 10 years ago

Ecco commented 11 years ago

This is a very small addition exposing more of the underlying C API. It can be useful to handle OOM cases (otherwise, by default v8 triggers a fatal error, and crashes).

reactormonk commented 11 years ago

Looks like a silent failure to me, the ugliest kind. If you want to ignore it, the API should be

begin
  V8::Context.new.eval('var a="0";while(true){a=a+a;}')
rescue IgnoreOutOfMemoryException
  # handle it
end
cowboyd commented 11 years ago

@Ecco Thanks for the PR!

@Tass does make a valid point, while I do think that it makes sense to expose the underlying API via V8::C, it seems like the most valuable thing would be to convert the crash into an exception. However, if you are explicitly telling V8 to ignore out of memory exceptions, then it's your fault if this silent failure bites you later on.

Any interest in converting the OOM situation to an exception? There should be a mechanism to do it via V8

Ecco commented 11 years ago

@Tass Agreed. But it's the way the V8 API itself was written…

I agree that having an explicit exception would be much better. Yet I'm not sure V8 even exposes the actual thing!

reactormonk commented 11 years ago

V8 running out of memory is treated as a fatal error by default. This means that the fatal error handler is called and that V8 is terminated.

and

static void v8::V8::SetFatalErrorHandler (FatalErrorCallback that)

Source No reference to this function anywhere in therubyracer.

With that, you can raise an error to ruby. But v8 is dead at that point, no way to recover it.

Ecco commented 11 years ago

Actually I did write a ruby binding to SetFatalErrorhandler. And deleted that code. Turns out it's pretty much useless because your program will still crash afterwards. You do get called back, but the process will crash afterwards nonetheless.

So that's why I thought 'ok, let's submit that simple pull request first. It does something very similar to other things in therubyracer, and it's already useful anyway'.

Now if you guys think this is useless, just don't merge this code, but I think this could be of use to other people.

cowboyd commented 10 years ago

I think a better approach would be to have a clean interface for ResourceConstraints and catch the exception thrown there.