Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Native node module crashes due to ObjectWrap::Unwrap<> returning a different pointer than what was passed to ObjectWrap::Wrap<> #961

Open
rkeithhill-keysight opened this issue Nov 4, 2023 · 0 comments

Comments

@rkeithhill-keysight
Copy link

We pass back a ptr to the object that implements our native node module's functionality to JavaScript using Nan::ObjectWrap::Wrap(). When we invoke a method in our native node module, that ptr is passed back in via the Nan::FunctionCallbackInfo<v8::Value> const& info parameter, which we get by using ObjectWrap::Unwrap<NodeAdapter>(info.Holder());.

Up through Electron 21.4.4., that Unwrap call has always returned the same pointer that was passed into the Wrap() call in our void NodeAdapter::New(const Nan::FunctionCallbackInfo<v8::Value>& info) method. This is important because there are instance fields on this object that get initialized at construct time that we expect to use later when the native node module's methods are invoked.

Starting with Electron 22.3.27, the ptr we get back from Unwrap is not the same as what was passed to Wrap() in the void NodeAdapter::New(const Nan::FunctionCallbackInfo<v8::Value>& info) method. This causes our renderer process to crash as soon as a native node module method is called and it tries to dereference the bad ptr it unwrapped.:

// New method
void NodeAdapter::New(const Nan::FunctionCallbackInfo<v8::Value>& info)
{
    // Adapter address returned here (during debug run) is: 0x000001d854075940
    auto* adapter = new NodeAdapter();

    adapter->Wrap(info.This());
    info.GetReturnValue().Set(info.This());
}

// log method
void NodeAdapter::log(Nan::FunctionCallbackInfo<v8::Value> const& info)
{
    // Problem first observed here.
    // Wrong address returned for this object (during same debug run): 0x00036b6000000000
    auto const adapter = ObjectWrap::Unwrap<NodeAdapter>(info.Holder());

    ...

    // CRASH - the first ptr deref crashes because we didn't get back the correct/original adapter address
    adapter->m_guiLogger->log(level, message);
}

I looked through changes between node v16.16 (Electron 21) and v16.17 (Electron 22) and I don't see anything that could be related. I see that Electron 21 introduced the V8 memory cage feature as discussed in this issue on crashing native node modules: electron/electron#35801. But the thing is, our native node module works on Electron 21 (unless the issue is intermittent). Also, we don't use ArrayBuffer explicitly but we do use the new keyword to create an instance of our NodeAdapter class (see below).

There is also this issue on a crashing renderer process starting at Electron 22 that may be related: electron/electron#36999.

We recently switched to context-aware by using NAN_MODULE_WORKER_ENABLED but that has been working fine in both Electron v14 and v21.

Our BrowserWindow (renderer) runs with nodeIntegration: true and contextIsolation: false. This native node module started on Electron 4 and has been working just fine until we tried to upgrade to Electron 22. Here's a redacted/abbreviated snapshot of our native node module code:

// NativeExtension.cpp:
// -----------------------------------------------------------------------
NAN_MODULE_INIT(InitAll)
{
    NodeAdapter::Init(target);
}

NAN_MODULE_WORKER_ENABLED(NativeExtension, InitAll);


// NodeAdapter.h
// -----------------------------------------------------------------------
#include <nan.h>
#include "GuiLogger.h"

struct NodeAdapter : public Nan::ObjectWrap
{
    static void Init(v8::Local<v8::Object> exports);

private:
    static void New(Nan::FunctionCallbackInfo<v8::Value> const& info);
    static void log(Nan::FunctionCallbackInfo<v8::Value> const& info);

    static Nan::Persistent<v8::Function> s_constructor;

    explicit NodeAdapter();

public:
    std::shared_ptr<GuiLogger> m_guiLogger;
}


// NodeAdapter.cpp:
// -----------------------------------------------------------------------
#include "NodeAdapter.h"

Nan::Persistent<v8::Function> NodeAdapter::s_constructor;
static constexpr auto ObjectName = "NodeAdapterWrapper";

// Init method
void NodeAdapter::Init(v8::Local<v8::Object> exports)
{
    v8::Local<v8::FunctionTemplate> tpl = Nan::New<v8::FunctionTemplate>(New);
    tpl->SetClassName(Nan::New(ObjectName).ToLocalChecked());
    tpl->InstanceTemplate()->SetInternalFieldCount(1);

    Nan::SetPrototypeMethod(tpl, "log", log);

    s_constructor.Reset(Nan::GetFunction(tpl).ToLocalChecked());
    Nan::Set(exports, Nan::New(ObjectName).ToLocalChecked(), Nan::GetFunction(tpl).ToLocalChecked());
}


// New method
void NodeAdapter::New(const Nan::FunctionCallbackInfo<v8::Value>& info)
{
    // Adapter address returned here (during debug run) is: 0x000001d854075940
    auto* adapter = new NodeAdapter();

    adapter->Wrap(info.This());
    info.GetReturnValue().Set(info.This());
}


// ctor
NodeAdapter::NodeAdapter()
{
    const auto guiLogger = new GuiLogger();
    m_guiLogger = guiLogger->getLogger();
}

// log method
void NodeAdapter::log(Nan::FunctionCallbackInfo<v8::Value> const& info)
{
    // Problem first observed here.
    // Wrong address returned for this object (during same debug run): 0x00036b6000000000
    auto const adapter = ObjectWrap::Unwrap<NodeAdapter>(info.Holder());

    ...

    // CRASH - the first ptr deref crashes because we didn't get back the correct/original adapter address
    adapter->m_guiLogger->log(level, message);
}


// Compiled with cmake-js & VS 2022 (MDd) with the following defines:
// -----------------------------------------------------------------------
//    -DV8_31BIT_SMIS_ON_64BIT_ARCH 
//    -DV8_COMPRESS_POINTERS
//    -DV8_REVERSE_JSARGS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant