API changes upcoming to make writing exception safe code more easy

[Michael Hablich]

Hey v8-users,

we're making big changes to the api (again).  We've found that our api makes it extremely difficult to write exception safe code.

Reference issue

https://code.google.com/p/v8/issues/detail?id=3929

Further description

Consider the following:

Local<Value> x = some_value;
s = x.ToString();
// in the current context, ToString might throw an exception, run out
of stack space, whatever...
// if that happens we just return an empty Local<String>
s->Anything() // Crash!

There's too much implicit stuff happening here, and it's bad.  It's especially bad if you use threads and rely on TerminateExecution(), which causes lots of v8 function to return empty handles unexpectedly.

To fix this, we will begin adding functions whose signature makes it impossible to make this mistake and we will deprecate and eventually remove all old functions.  The new functions will in general come in
two flavours:

functions like:
Local<String> ToString(Isolate*);

will become:
MaybeLocal<String> ToString(Local<Context>);

and functions like:
double NumberValue();

will become:
Maybe<bool> NumberValue(Local<Context>);

The Maybe values ensure at compilation time that the value returned is checked by the embedder, and the explicit Context makes it clear that a context is actually required and that javascript may be executed in that context.

Timeline
We will add the new API parts gradually to the codebase. When this is finished there will be approx. 6 weeks of time where the old and the new API will exist side-by-side. This is a good time to make the adjustments in your embedding code. After this period the old API will get removed.

We apologize in advance for any trouble this may cause, but we're positive that this change will reduce crash rates for practically all v8 embedders.

Cheers,

Michael Hablich

[Ben Noordhuis]

I think this should read `Maybe<double> NumberValue(Local<Context>)`?

> The Maybe values ensure at compilation time that the value returned is
> checked by the embedder, and the explicit Context makes it clear that a
> context is actually required and that javascript may be executed in that
> context.
>
> Timeline
> We will add the new API parts gradually to the codebase. When this is
> finished there will be approx. 6 weeks of time where the old and the new API
> will exist side-by-side. This is a good time to make the adjustments in your
> embedding code. After this period the old API will get removed.
>
> We apologize in advance for any trouble this may cause, but we're positive
> that this change will reduce crash rates for practically all v8 embedders.
>
> Cheers,
> Michael Hablich

Can you provide guidelines on what context to use in API callbacks? I
see that src/api.cc uses Isolate::GetCurrentContext() in a number of
places. When would you use the current context, when
Isolate::GetCallingContext(), etc.?

[Jochen Eisinger]

correct

 

> The Maybe values ensure at compilation time that the value returned is
> checked by the embedder, and the explicit Context makes it clear that a
> context is actually required and that javascript may be executed in that
> context.
>
> Timeline
> We will add the new API parts gradually to the codebase. When this is
> finished there will be approx. 6 weeks of time where the old and the new API
> will exist side-by-side. This is a good time to make the adjustments in your
> embedding code. After this period the old API will get removed.
>
> We apologize in advance for any trouble this may cause, but we're positive
> that this change will reduce crash rates for practically all v8 embedders.
>
> Cheers,
> Michael Hablich

Can you provide guidelines on what context to use in API callbacks?  I
see that src/api.cc uses Isolate::GetCurrentContext() in a number of
places.  When would you use the current context, when
Isolate::GetCallingContext(), etc.?

We'll put a getter for the context to use on FunctionCallbackInfo and ReturnValue.

best

-jochen

 

--
--
v8-users mailing list
v8-u...@googlegroups.com
http://groups.google.com/group/v8-users
---
You received this message because you are subscribed to the Google Groups "v8-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to v8-users+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Adam Klein]

I'm curious about this bit of the design: why was it decided to add an argument here rather than require a Context to be entered (via Context::Scope)?

Rather late to the discussion, I know. Apologies

- Adam

 

Timeline
We will add the new API parts gradually to the codebase. When this is finished there will be approx. 6 weeks of time where the old and the new API will exist side-by-side. This is a good time to make the adjustments in your embedding code. After this period the old API will get removed.

We apologize in advance for any trouble this may cause, but we're positive that this change will reduce crash rates for practically all v8 embedders.

Cheers,

Michael Hablich

--

--
v8-users mailing list
v8-u...@googlegroups.com
http://groups.google.com/group/v8-users
---
You received this message because you are subscribed to the Google Groups "v8-users" group.

To unsubscribe from this group and stop receiving emails from it, send an email to v8-users+u...@googlegroups.com.

[Jochen Eisinger]

In C++ you can't overload a method with just a different return type. Since making the context explicit instead of hoping that the right one is entered is nice to have, it was a handy way to change the return type.

[Adam Klein]

Interesting. I agree it's "nice to have" in a debugging sort of way, both for embedders and v8 itself (no weird crashes due to people forgetting to enter a Context). But I wonder if we're now in a sort of inconsistent state, where some APIs require entering a Context before calling and others take this as an argument. Are we planning to move more of the API towards _always_ passing a Context?

[Dan Carney]

I've already converted most of the functions to take a context that might use one internally.  Part of the change is to make explicit what v8 maybe should never have made implicit.  I've found a number of places when calls are made that require a context internally on some uncommon paths but we're using them without one.

[Adam Klein]

Excellent to hear, this sounds like an overall win (I say this as someone who in my early V8 interactions often found myself surprised by the need of a Context in cases I'd have guessed didn't need one).