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

Import crashes in electron renderer process during reload [macOs] #4491

Open
itsbjoern opened this issue Apr 13, 2022 · 3 comments
Open

Import crashes in electron renderer process during reload [macOs] #4491

itsbjoern opened this issue Apr 13, 2022 · 3 comments

Comments

@itsbjoern
Copy link

How frequently does the bug occur?

All the time

Description

When importing Realm in an electron renderer process with nodeIntegration enabled, the renderer process will crash every time when reloading the BrowserWindow.

This is enough to cause it.

    // preload.js
    const Realm = require('realm');

It appears to me that the root cause is something to do with the re-import of the NAPI module.
Creating a Persistent NAPI object in node_class.hpp fails due to the reference in napi trying to reset and removing itself.

The crash is initiated by:

ObjectSetPrototypeOf = Napi::Persistent(setPrototypeOf);

And ultimately causes a segfault at:
https://github.com/nodejs/node-addon-api/blob/9bea434326d5e6c6fa355a51b6f232a503521a21/napi-inl.h#L2975

napi_status status = napi_delete_reference(_env, _ref);

I am not pretending to actually know anything about this as I'm not familiar with node native modules or c++ really, but I found an issue from 2018 that referenced problems with static references (even though supposedly fixed).
nodejs/node-addon-api#162.

For what it's worth I was able to produce a version that doesn't crash anymore (obviously without any actual working functionality) by building a dummy of napi_init where the persistent object is scoped to the function. Now I realise that this might not actually help anything but I thought I'd report the find.

static void napi_init(Napi::Env env, Napi::Object exports)
{
    Napi::FunctionReference test;
    auto setPrototypeOf = env.Global().Get("Object").As<Napi::Object>().Get("setPrototypeOf").As<Napi::Function>();
    test = Napi::Persistent(setPrototypeOf);

    // node_class_init(env);

    // Napi::Function realm_constructor = js::RealmClass<Types>::create_constructor(env);

    // std::string name = realm_constructor.Get("name").As<Napi::String>();
    // exports.Set(Napi::String::New(env, name), realm_constructor);
}

I'm reporting this here because I'm unsure who is actually responsible for a fix, but it seems like that the either it is impossible to store the function references as static objects like done here or there is an issue with napi not handling that case correctly even though it should.

Let me know if you need any more information, cheers.

Stacktrace & log output

Thread 0 Crashed:: CrRendererMain Dispatch queue: com.apple.main-thread
0   Electron Framework            	       0x11933c8f0 napi_delete_reference + 48
1   realm.node                    	       0x10c33cb8f Napi::Reference<Napi::Function>::Reset() + 47 (napi-inl.h:2787)
2   realm.node                    	       0x10c33cafd Napi::Reference<Napi::Function>::operator=(Napi::Reference<Napi::Function>&&) + 29 (napi-inl.h:2706)
3   realm.node                    	       0x10c33c701 Napi::FunctionReference::operator=(Napi::FunctionReference&&) + 49 (napi-inl.h:3038)
4   realm.node                    	       0x10c33b5f5 realm::node::node_class_init(Napi::Env) + 165 (node_class.hpp:63)
5   realm.node                    	       0x10c33b448 realm::node::napi_init(Napi::Env, Napi::Object) + 40 (node_init.cpp:35)
6   realm.node                    	       0x10c33afe5 NAPI_Init(Napi::Env, Napi::Object) + 53 (node_init.cpp:48)
7   realm.node                    	       0x10c33b10b Napi::RegisterModule(napi_env__*, napi_value__*, Napi::Object (*)(Napi::Env, Napi::Object))::'lambda'()::operator()() const + 91 (napi-inl.h:376)
8   realm.node                    	       0x10c33b011 napi_value__* Napi::details::WrapCallback<Napi::RegisterModule(napi_env__*, napi_value__*, Napi::Object (*)(Napi::Env, Napi::Object))::'lambda'()>(Napi::RegisterModule(napi_env__*, napi_value__*, Napi::Object (*)(Napi::Env, Napi::Object))::'lambda'()) + 17 (napi-inl.h:75)
9   realm.node                    	       0x10c33af9e Napi::RegisterModule(napi_env__*, napi_value__*, Napi::Object (*)(Napi::Env, Napi::Object)) + 78 (napi-inl.h:375)
10  realm.node                    	       0x10c33af44 __napi_NAPI_Init(napi_env__*, napi_value__*) + 36 (node_init.cpp:52)
11  Electron Framework            	       0x11933fb03 node::AsyncResource::CallbackScope::CallbackScope(node::AsyncResource*) + 1651

Can you reproduce the bug?

Yes, always

Reproduction Steps

Import Realm in a electron renderer process while nodeIntegration is enabled and reload the window.

Version

10.14.0

What SDK flavour are you using?

Local Database only

Are you using encryption?

No, not using encryption

Platform OS and version(s)

macOS 12.1

Build environment

Which debugger for React Native: ..

Cocoapods version

No response

@tomduncalf
Copy link
Contributor

Hi @BFriedrichs, could you clarify how you are "reloading the BrowserWindow"? I'm not an expert on Electron, if you're able to supply a simple reproduction project and instructions that would be really helpful! Thanks

@sync-by-unito sync-by-unito bot added the Waiting-For-Reporter Waiting for more information from the reporter before we can proceed label Apr 14, 2022
@itsbjoern
Copy link
Author

itsbjoern commented Apr 14, 2022

Hey, I think this is the most minimal example I could come up with.

https://github.com/BFriedrichs/realm-issue4491

Edit: Also, to answer your question. Literally any action that refreshes the embedded browser contents which causes 'preload.js' to be executed again causes this issue as far as I can tell. That includes window.location.reload() or pressing CMD+R but also live-reload features as you would expect from various webpack related projects.

@github-actions github-actions bot added Needs-Attention Reporter has responded. Review comment. and removed Waiting-For-Reporter Waiting for more information from the reporter before we can proceed labels Apr 14, 2022
@tomduncalf
Copy link
Contributor

Great, thanks for the repro and I can confirm that it crashes here. We'll have to do some investigation into whether this is something we can fix or provide a workaround for, we will update this ticket when we've been able to take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants