Segmentation fault when using fly server (node version >=10.4) #101
Comments
|
Which OS and node version was this? If you can try a new node version, |
nick-potts
changed the title
fly server
|
macOS 10.13.5, node/10.4.1 |
|
10.3 works, but I think anything higher segfaults. I believe it’s an issue
with isolated-vm, a package we use.
I can open a ticket there.
…--
Jérôme Gravel-Niquet
On July 2, 2018 at 8:53:59 PM, Nick ***@***.***) wrote:
macOS 10.13.5, node/10.4.1
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#101 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AACpPf37HgmT0aaQ_14GOhtL4pewyQQJks5uCsCngaJpZM4VAH5i>
.
|
|
Yep, that's my conclusion too. |
|
@laverdet any ideas? |
|
@nick-potts you need to |
|
That's fixed the error above, now I'm back to the seg fault. I've joined the slack channel. |
|
The |
|
Correction: I didn't submit a patch but the problem is clear :) |
|
@nick-potts did you happen to nuke node_modules and reinstall from npm after you downgraded to 10.3? I just tried a fresh 10.3 install + fresh npm install to double check and it works here. |
|
Further correction: now that I remember the issue with that package it's not that segfault-handler causes segfaults, it just makes them harder to track. Could you please drop that package and post the resulting stack trace? |
|
10.3 + |
mrkurt
changed the title
|
@laverdet I removed the segfault handler, installed Here's what |
|
I've been looking into these errors and I think they have to do with v8's snapshot feature. This example will fail while using a debug build of node: 'use strict';
let ivm = require('isolated-vm');
let snapshot = ivm.Isolate.createSnapshot([ { code: `
(function() {
class Foo {};
this.fn = function bootstrap() {
return Foo;
};
})();
` } ]);I'll need to dig a lot deeper to find out exactly what is going wrong. |
|
Oh! That's interesting. I can try some stuff without snapshots tonight and try to confirm that. |
mrkurt
added a commit
that referenced
this issue
Jul 17, 2018
|
@laverdet that was hugely helpful, thank you. We can just skip snapshots when node is >=10.4.0 for now. |
mrkurt
added a commit
that referenced
this issue
Jul 18, 2018
|
I think this issue is fixed in latest v8, but it's hard to tell because getting latest v8 to work with current nodejs is not trivial. The two fixes that are relevant to the snapshot issue are: So either these two patches will need to be backported or we'll have to wait until nodejs picks up v8 6.9.186. Right now nodejs is using 6.7.288, so I would expect snapshots to be broken for a while unless we push the team to backport these commits. 39496a95 in particular is a pretty hairy patch that touches a lot of files so I'm not sure the maintainers would feel like it. On the other hand I think that maybe only 22116dd6 is necessary. I've forked v10.6.0 and backported both patches (and an additional dependency patch) if any of you want to try it out-- https://github.com/laverdet/node/tree/snapshots Edit: Github posted my comment before I could finish. Anyway, I've done some cursory testing with my branch and it seemed like it might work but I ran into some other issues that I don't have time to troubleshoot. I'll be offline tonight and for the day tomorrow but I wanted to update with my findings. |
|
@laverdet that's super helpful, thank you. I think we're good just not using snapshots on those node versions for now. I'll give your fork a shot next week sometime and see if it helps, though! |
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Just using the example code from the docs.
The text was updated successfully, but these errors were encountered: