Skip to content

Commit

Permalink
perform some renames to better reflect semantics
Browse files Browse the repository at this point in the history
  • Loading branch information
Gabriel Schulhof committed Jan 2, 2020
1 parent c4473ea commit 759800a
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 27 deletions.
32 changes: 16 additions & 16 deletions napi-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -2702,35 +2702,35 @@ inline Object FunctionReference::New(const std::vector<napi_value>& args) const
// CallbackInfo class
////////////////////////////////////////////////////////////////////////////////

class ObjectWrapCleanup {
class ObjectWrapConstructionContext {
public:
ObjectWrapCleanup(CallbackInfo* info) {
info->_object_wrap_cleanup = this;
ObjectWrapConstructionContext(CallbackInfo* info) {
info->_objectWrapConstructionContext = this;
}

static inline void MarkWrapOK(const CallbackInfo& info) {
if (info._object_wrap_cleanup == nullptr) {
Napi::Error::Fatal("ObjectWrapCleanup::MarkWrapOK",
"_object_wrap_cleanup is NULL");
static inline void SetObjectWrapped(const CallbackInfo& info) {
if (info._objectWrapConstructionContext == nullptr) {
Napi::Error::Fatal("ObjectWrapConstructionContext::SetObjectWrapped",
"_objectWrapConstructionContext is NULL");
}
info._object_wrap_cleanup->_wrap_worked = true;
info._objectWrapConstructionContext->_objectWrapped = true;
}

inline void RemoveWrap(const CallbackInfo& info) {
if (_wrap_worked) {
inline void Cleanup(const CallbackInfo& info) {
if (_objectWrapped) {
napi_status status = napi_remove_wrap(info.Env(), info.This(), nullptr);

// There's already a pending exception if we are at this point, so we have
// no choice but to fatally fail here.
NAPI_FATAL_IF_FAILED(status,
"ObjectWrapCleanup::RemoveWrap",
"ObjectWrapConstructionContext::Cleanup",
"Failed to remove wrap from unsuccessfully "
"constructed ObjectWrap instance");
}
}

private:
bool _wrap_worked = false;
bool _objectWrapped = false;
};

inline CallbackInfo::CallbackInfo(napi_env env, napi_callback_info info)
Expand Down Expand Up @@ -3140,7 +3140,7 @@ inline ObjectWrap<T>::ObjectWrap(const Napi::CallbackInfo& callbackInfo) {
status = napi_wrap(env, wrapper, this, FinalizeCallback, nullptr, &ref);
NAPI_THROW_IF_FAILED_VOID(env, status);

ObjectWrapCleanup::MarkWrapOK(callbackInfo);
ObjectWrapConstructionContext::SetObjectWrapped(callbackInfo);
Reference<Object>* instanceRef = this;
*instanceRef = Reference<Object>(env, ref);
}
Expand Down Expand Up @@ -3716,21 +3716,21 @@ inline napi_value ObjectWrap<T>::ConstructorCallbackWrapper(

napi_value wrapper = details::WrapCallback([&] {
CallbackInfo callbackInfo(env, info);
ObjectWrapCleanup cleanup(&callbackInfo);
ObjectWrapConstructionContext constructionContext(&callbackInfo);
#ifdef NAPI_CPP_EXCEPTIONS
try {
new T(callbackInfo);
} catch (const Error& e) {
// Re-throw the error after removing the failed wrap.
cleanup.RemoveWrap(callbackInfo);
constructionContext.Cleanup(callbackInfo);
throw e;
}
#else
T* instance = new T(callbackInfo);
if (callbackInfo.Env().IsExceptionPending()) {
// We need to clear the exception so that removing the wrap might work.
Error e = callbackInfo.Env().GetAndClearPendingException();
cleanup.RemoveWrap(callbackInfo);
constructionContext.Cleanup(callbackInfo);
e.ThrowAsJavaScriptException();
delete instance;
}
Expand Down
6 changes: 3 additions & 3 deletions napi.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ namespace Napi {
class CallbackInfo;
class TypedArray;
template <typename T> class TypedArrayOf;
class ObjectWrapCleanup;
class ObjectWrapConstructionContext;

typedef TypedArrayOf<int8_t> Int8Array; ///< Typed-array of signed 8-bit integers
typedef TypedArrayOf<uint8_t> Uint8Array; ///< Typed-array of unsigned 8-bit integers
Expand Down Expand Up @@ -1403,7 +1403,7 @@ namespace Napi {

class CallbackInfo {
public:
friend class ObjectWrapCleanup;
friend class ObjectWrapConstructionContext;
CallbackInfo(napi_env env, napi_callback_info info);
~CallbackInfo();

Expand All @@ -1429,7 +1429,7 @@ namespace Napi {
napi_value _staticArgs[6];
napi_value* _dynamicArgs;
void* _data;
ObjectWrapCleanup* _object_wrap_cleanup;
ObjectWrapConstructionContext* _objectWrapConstructionContext;
};

class PropertyDescriptor {
Expand Down
9 changes: 1 addition & 8 deletions test/objectwrap_constructor_exception.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,8 @@ const assert = require('assert');

const test = (binding) => {
const { ConstructorExceptionTest } = binding.objectwrapConstructorException;
let gotException = false;
try {
new ConstructorExceptionTest();
} catch (anException) {
gotException = true;
}
assert.throws(() => (new ConstructorExceptionTest()));
global.gc();

assert.strictEqual(gotException, true);
}

test(require(`./build/${buildType}/binding.node`));
Expand Down

0 comments on commit 759800a

Please sign in to comment.