Correct abstraction for try/catch

[mhdawson]

• Version: ALL
• Platform: ALL
• Subsystem: API

Comment from Ben in : nodejs/node-eps#20 (comment)

I think most JS engines have an 'is exception pending?' function; V8 is the outlier with its TryCatch construct. API strawman:

// Wrapper around a v8::TryCatch. Dummy with other engines.
typedef struct napi_trycatch { struct napi_trycatch *v; } napi_trycatch;
NODE_EXTERN void napi_trycatch_new(napi_env e, napi_trycatch *trycatch);
// Returns exception object or undefined.
NODE_EXTERN napi_value napi_trycatch_exception(napi_env e, napi_trycatch trycatch);
NODE_EXTERN void napi_trycatch_delete(napi_env e, napi_trycatch trycatch);

[gabrielschulhof]

The V8 documentation claims that the TryCatch should be stack-allocated, so I'm not sure if we can go with the new/delete paradigm. If we can't, we may have to provide a way of calling a function that also catches exceptions, like maybe with an out parameter:

napi_value
napi_call_function_with_exception(napi_env e,
                                  napi_value scope,
                                  napi_value func,
                                  int argc,
                                  napi_value* argv,
                                  napi_value* exception);

Then, applications can simply re-throw the exception if our implementation sets it.

OTOH, if we can heap-allocate, IINM in the V8 implementation we have to remember to v8::TryCatch::ReThrow() the exception in napi_trycatch_delete() if napi_trycatch_exception() has not been called because V8 automatically removes the exception state if a TryCatch is within scope. So, in V8 we need at least one additional piece of data in addition to the v8::TryCatch itself: A sticky bit exception_retrieved which will be set in napi_trycatch_exception() to indicate that we need not call ReThrow() in napi_trycatch_delete(), because the exception has been retrieved.

This way, we have the JsGetAndClearException() semantics in napi_trycatch_exception(), but also the need to have an object that bookends the transaction that may throw à la v8::TryCatch.

Unfortunately this means that, if somebody leaks the napi_trycatch by failing to call napi_trycatch_delete() then V8 and ChakraCore will behave differently because, in V8, the exception will have been caught, whereas in ChakraCore it will have been left in place.

[aruneshchandra]

@fhinkel - can you comment on this ?

[gabrielschulhof]

During our last meeting, I learned that there are other V8 structures that are also documented as having to be stack-allocated yet work fine when heap-allocated. So, I went ahead and created an implementation of @mhdawson's API with a heap-allocated v8::TryCatch and the boolean flag as I described in my comment. I also created an implementation for ChakraCore.

[hashseed]

The reason v8::TryCatch ought to be stack-allocated is that nested v8::TryCatch scopes build a chain that reflect this nesting. Heap-allocating would be possible, but must make sure that the allocation/deallocation order is kept. Otherwise things might break in weird ways. Letting it escape via proposed API seems an easy source for bugs.

[fhinkel]

@hashseed do you think the API could abstract it in such a way that it can't be used incorrectly?

[hashseed]

Well you could, maybe in debug mode, track the nesting inside napi_trycatch_* and make sure we can only call napi_trycatch_delete on the last v8::TryCatch in the chain. But if the user would use v8::TryCatch intermixed with napi_trycatch_*, we could still deallocate out of order.

[gabrielschulhof]

What about implementing the ChakraCore abstraction instead? We could surround all calls into JS (napi_call_function()) with a v8::TryCatch, and store a possible exception in a Persistent. Then, that would be the "last known exception", and a call to napi_trycatch_exception() would return it and clear the persistent.

Otherwise, if nobody calls napi_trycatch_exception(), napi_trycatch_delete() would throw the exception present in the persistent, if any.

Is there a big performance penalty when surrounding a call into JS with a v8::TryCatch?