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

Crash when node-ffi is required #2680

Closed
rosen-vladimirov opened this issue Sep 1, 2015 · 24 comments
Closed

Crash when node-ffi is required #2680

rosen-vladimirov opened this issue Sep 1, 2015 · 24 comments

Comments

@rosen-vladimirov
Copy link

Hi,

I'm trying to use node-ffi as dependency in the electron, but the require('ffi') leads to crash. Please note I'm using v2 branch of node-ffi, as it has support for iojs 3.x and this is the branch from which the new version should be released in npm very soon. The v2 branch can be required (after successful build of course) without any issues in iojs 2.x, iojs 3.x, node 0.12.x, but it fails in electron. I've noticed you are using a fork of node and there are some changes in it, so I've tried using older electron version - 0.29.1.The ffi module is successfully required with it, so I've checked the differences in your node's fork - the only one I see is this commit - could you share more details about it and could it cause the problem?

I've tried my app with electron 0.31.0 and 0.31.1, but without success.
You can find a sample application that reproduces the issue here. After cloning it, just run installAndStart script with target and architecture paramters, for example:

./installAndStart.sh 0.31.0 x64

And the modules will be rebuild with correct headers. At the end the electron should start, but it just crashes.

I've tried rebuilding your node's fork without the latest commit, but I'm not sure how to produce dll from it in order to replace it in my electron's installation.

Could you check what's causing this crash? I would have contacted node-ffi's authors, but their version is working fine with real iojs, it looks like the latest commit in your iojs's fork is causing the issue.

Thanks in advance for your help!

@etiktin
Copy link
Contributor

etiktin commented Sep 1, 2015

Did you follow the instructions mentioned here?
You need to build native modules with Electron's headers. The easiest way to do that is to use electron-rebuild (you can find details in the instructions link).

@rosen-vladimirov
Copy link
Author

I've followed them very carefully. Also the installAndStart script that I've included in the project is doing this - https://github.com/rosen-vladimirov/myElectronApp/blob/master/installAndStart.sh#L17
The building is correct, as when I do not execute the mentioned lines, I get a message box with Dynamic Link Library initalization failed error. In my case, the electron just crashes - no message, no log... The same dependency is working fine with real iojs :)

@zcbenz
Copy link
Contributor

zcbenz commented Sep 2, 2015

Your script probably didn't recompile all the native dependencies, I suggest using npm by setting the environment variables or just electron-rebuild instead.

@anaisbetts
Copy link
Contributor

I get a message box with Dynamic Link Library initalization failed error

This means that electron-rebuild isn't working, you're using the iojs version.

@rosen-vladimirov
Copy link
Author

Thank you for your suggestions. I've added the script just for easier reproduction case. I've manually rebuild all modules and as I've said, I'm successfully rebuilding the same package and using it with iojs. @paulcbetts the error message about Dynamic Link Library is not the one that concerns me. I'm NOT receiving it after successfull rebuild. My electron app crashes directly after successfully rebuilding ALL native dependencies and requiring ffi.
@zcbenz my script works fine with electron 0.29.1 and my app starts correctly, so I'm pretty sure the problem is not in it.
Could you try the application I've provided. It's really simple - just require("ffi"). You can use the script for easier rebuilding of the native modules, but if you think I've made mistake, you can rebuild ffi and its ref dependency on your own.
I'm looking forward to hearing from you.

@rosen-vladimirov
Copy link
Author

Hi, I've also tried the latest electron - 0.31.2, but it's still crashing 😢

@Mitko-Kerezov
Copy link

This is a major blocking issue for us I'd really like to see this fixed too

@hokein
Copy link
Contributor

hokein commented Sep 3, 2015

Currenly node-ffi doesn't support iojs v3 yet, the upgrade work is here: node-ffi/node-ffi#231. So you need to wait for the new release version of node-ffi or use electron v0.30.x as a workaround.

@hokein hokein added the blocked label Sep 3, 2015
@rosen-vladimirov
Copy link
Author

Hi @hokein ,
Thank you for pointing this Pull Request. However if you take a closer look at the issue I've reported, I'm using v2 branch of node-ffi. This branch has support for iojs 3 and this branch will be used for version 2.x of node-ffi. Also this branch is working fine on node 0.10, 0.12, iojs 2 and iojs 3 as it can be easily seen here.
I cannot use electron v0.30.x due to some other issues related to the v8 version that these versions use.
We are really blocked by this issue (I like the tag you've set). Could you at least share some information why do you need this commit and how can we build the node as dll, maybe we can work together in order to fix the problem.
Also aren't you worried that when node-ffi is released from v2 branch all applications that are using it will fail in electron?

@hokein
Copy link
Contributor

hokein commented Sep 4, 2015

@rosen-vladimirov

However if you take a closer look at the issue I've reported, I'm using v2 branch of node-ffi. This branch has support for iojs 3 and this branch will be used for version 2.x of node-ffi.

Oh, I have missed it before. I can reproduce it too, it's probably a bug of Electron.

why do you need this commit and how can we build the node as dll, maybe we can work together in order to fix the problem.

We do need this commit for embedded blink allocator in node's buffer. Because iojs reimplements node buffer in v0.3, and it starts to manage the memory of buffer, which conflicts with Blink's behavior.
It will cause electron to crash if we remove this commit.

Instead of rebuilding and replace the node library for electron-prebuilt v0.31, I suggest you to build the whole electron, because there is extra build-related work on the node library(e.g. change the path of libnode on OS X). The following steps are:

  1. Clone electron repo, and checkout v0.31.0 tag.
  2. run ./script/bootstrap -d
  3. run ./script/build -c D

@hokein
Copy link
Contributor

hokein commented Sep 4, 2015

A crash stack trace:

rashed Thread:        0  Dispatch queue: com.apple.main-thread

Exception Type:        EXC_BAD_ACCESS (SIGSEGV)
Exception Codes:       KERN_INVALID_ADDRESS at 0x0000000000000000

VM Regions Near 0:
--> 
    __TEXT                 00000001070b7000-00000001070b8000 [    4K] r-x/rwx SM=COW  /Users/USER/*/Electron.app/Contents/MacOS/Electron

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   ffi_bindings.node               0x0000000110491fba ffi_prep_cif_core + 61
1   ffi_bindings.node               0x000000011049211a ffi_prep_cif + 23
2   ffi_bindings.node               0x000000011048ff93 FFI::FFIPrepCif(Nan::FunctionCallbackInfo<v8::Value> const&) + 469
3   ffi_bindings.node               0x0000000110490a18 Nan::imp::FunctionCallbackWrapper(v8::FunctionCallbackInfo<v8::Value> const&) + 131
4   libnode.dylib                   0x000000010b15b0bf v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) + 159
5   libnode.dylib                   0x000000010b17fa85 0x10ade2000 + 3791493
6   libnode.dylib                   0x000000010b18279d 0x10ade2000 + 3803037
7   ???                             0x000035ce61b07f9b 0 + 59160518492059
8   ???                             0x000035ce61cb97e6 0 + 59160520267750
9   ???                             0x000035ce61cb8b44 0 + 59160520264516
10  ???                             0x000035ce61b063b5 0 + 59160518484917
11  ???                             0x000035ce61cb7e1e 0 + 59160520261150
12  ???                             0x000035ce61b31113 0 + 59160518660371
13  ???                             0x000035ce61c53e70 0 + 59160519851632
14  ???                             0x000035ce61c50245 0 + 59160519836229
15  ???                             0x000035ce61c4d152 0 + 59160519823698
16  ???                             0x000035ce61c3fe65 0 + 59160519769701
17  ???                             0x000035ce61b063b5 0 + 59160518484917
18  ???                             0x000035ce61c5636e 0 + 59160519861102
19  ???                             0x000035ce61c56156 0 + 59160519860566
20  ???                             0x000035ce61c8ed1c 0 + 59160520092956
21  ???                             0x000035ce61b31113 0 + 59160518660371
22  ???                             0x000035ce61c53e70 0 + 59160519851632
23  ???                             0x000035ce61c50245 0 + 59160519836229
24  ???                             0x000035ce61c4d152 0 + 59160519823698
25  ???                             0x000035ce61c3fe65 0 + 59160519769701
26  ???                             0x000035ce61b063b5 0 + 59160518484917
27  ???                             0x000035ce61c5636e 0 + 59160519861102
28  ???                             0x000035ce61c56156 0 + 59160519860566
29  ???                             0x000035ce61c8dd63 0 + 59160520088931
30  ???                             0x000035ce61b31113 0 + 59160518660371
31  ???                             0x000035ce61c53e70 0 + 59160519851632
32  ???                             0x000035ce61c50245 0 + 59160519836229
33  ???                             0x000035ce61c4d152 0 + 59160519823698
34  ???                             0x000035ce61c3fe65 0 + 59160519769701
35  ???                             0x000035ce61c8625a 0 + 59160520057434
36  ???                             0x000035ce61b31113 0 + 59160518660371
37  ???                             0x000035ce61c53e70 0 + 59160519851632
38  ???                             0x000035ce61c50245 0 + 59160519836229
39  ???                             0x000035ce61c4d152 0 + 59160519823698
40  ???                             0x000035ce61c3fe65 0 + 59160519769701
41  ???                             0x000035ce61c55daa 0 + 59160519859626
42  ???                             0x000035ce61c55f32 0 + 59160519860018
43  ???                             0x000035ce61b31113 0 + 59160518660371
44  ???                             0x000035ce61c53e70 0 + 59160519851632
45  ???                             0x000035ce61c50245 0 + 59160519836229
46  ???                             0x000035ce61c4d152 0 + 59160519823698
47  ???                             0x000035ce61c3fe65 0 + 59160519769701
48  ???                             0x000035ce61c3f5a3 0 + 59160519767459
49  ???                             0x000035ce61b36ca9 0 + 59160518683817
50  ???                             0x000035ce61b342d5 0 + 59160518673109
51  ???                             0x000035ce61b2e5bd 0 + 59160518649277
52  ???                             0x000035ce61b1e2c2 0 + 59160518582978
53  libnode.dylib                   0x000000010b29268e 0x10ade2000 + 4916878
54  libnode.dylib                   0x000000010b146a24 v8::Function::Call(v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) + 276
55  libnode.dylib                   0x000000010adf3d64 node::LoadEnvironment(node::Environment*) + 324
56  Electron Framework              0x000000010716a413 atom::NodeBindings::LoadEnvironment(node::Environment*) + 67
57  Electron Framework              0x00000001071243eb atom::AtomBrowserMainParts::PostEarlyInitialization() + 203
58  Electron Framework              0x00000001076dd39a 0x1070be000 + 6419354
59  Electron Framework              0x00000001076e2e93 0x1070be000 + 6442643
60  Electron Framework              0x00000001076dcce7 0x1070be000 + 6417639
61  Electron Framework              0x0000000107673aa2 0x1070be000 + 5986978
62  Electron Framework              0x0000000107673146 0x1070be000 + 5984582
63  Electron Framework              0x00000001070c02cd AtomMain + 77
64  com.github.electron             0x00000001070b7eea main + 58
65  libdyld.dylib                   0x00007fff93d0c5c9 start + 1

@zcbenz
Copy link
Contributor

zcbenz commented Sep 4, 2015

node-ffi stores pointer by passing it to Buffer (which is ArrayBuffer underlying), however in Chromium users are not allowed to create ArrayBuffer with their own pointers, and Electron works around it by copying the content pointed by the pointer to the ArrayBuffer allocated by Chromium. Unfortunately this resulted in node-ffi not getting the original pointer.

I don't think it is fixable in Electron, but it should be very easy to fix it in node-ffi by using another way of storing pointer.

@rosen-vladimirov
Copy link
Author

Thank you for taking a look at this issue!!!
@zcbenz

 in Chromium users are not allowed to create ArrayBuffer with their own pointers

Is this change in latest Chromium versions, as the same code from ffi is working in older versions of Electron?

@zcbenz
Copy link
Contributor

zcbenz commented Sep 4, 2015

Is this change in latest Chromium versions, as the same code from ffi is working in older versions of Electron?

It has always been required by Chromium, the problem is caused by Buffer being reimplemented as ArrayBuffer after io.js 3.0.0.

@rosen-vladimirov
Copy link
Author

Just FYI, ffi 2.0.0 had been released and as we've already discussed - it is not working in electron. We are still not sure how to resolve the issue, so in case you have any ideas, we'll be glad to hear them.

Thanks in advance!

@etiktin
Copy link
Contributor

etiktin commented Sep 5, 2015

@zcbenz, said that fixing the issue in node-ffi should be easy. So I guess we should open an issue there and figure out if they are willing to change the way they manage the pointers.

@rosen-vladimirov
Copy link
Author

@etiktin, here it is - node-ffi/node-ffi#236
Let's see what will be the answer there, but as the problem is caused by commit in electron's iojs fork, I'm not sure if they will have time to investigate the case.
@zcbenz , I'm not very good with C++, so could you give more details for your idea how to fix node-ffi?

Thank you all for your cooperation!

@thomasjo
Copy link
Contributor

thomasjo commented Sep 6, 2015

Looks like the underlying problem was resolved in nodejs/node#2487, which was released in io.js v3.2.0.

@zcbenz I guess the problem can be resolved by upgrading Electron's io.js to v3.2.0 or later? Alternatively patching with nodejs/node@07c0667.

@zcbenz
Copy link
Contributor

zcbenz commented Sep 7, 2015

The crash in io.js is not the same one in Electron, but I think I have found a way to fix that in Electron without patching node-ffi.

@anaisbetts
Copy link
Contributor

@zcbenz This is awesome - node-ffi is definitely a module we can't easily remove in the Slack app

@rosen-vladimirov
Copy link
Author

Hey guys,
Thank you very much for your efforts. I'll test the fix once you include it in your official release. Can we expect it in 31.3 or 33.0 (I'm not sure of your naming convention :) )

Thanks again!

@zcbenz
Copy link
Contributor

zcbenz commented Sep 7, 2015

It is going to be included in v0.32.0.

@anaisbetts
Copy link
Contributor

Bless u @zcbenz, and thanks @rosen-vladimirov for chasing down all of this useful history and debugging information, super helpful!

@goel-sunny
Copy link

i am still facing the issue . my electron application crashes eventually , don't have any clue why it is crashing , and their is no fix time of crashing, it crashes at any point at any time .

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

8 participants