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

takeSnapshot segfaults on Node 8 #112

Open
cxreg opened this issue Jun 6, 2017 · 14 comments
Open

takeSnapshot segfaults on Node 8 #112

cxreg opened this issue Jun 6, 2017 · 14 comments

Comments

@cxreg
Copy link

cxreg commented Jun 6, 2017

Attempting to take a snapshot segfaults

$ node -v
v8.0.0
$ node -e 'require("v8-profiler").takeSnapshot()'
Segmentation fault (core dumped)

I built a Debug build and got this stacktrace

$ /home/daveo/src/node/out/Debug/node -e 'require("v8-profiler").takeSnapshot()'


#
# Fatal error in ../deps/v8/src/api.h, line 345
# Check failed: allow_empty_handle || that != __null.
#

==== C stack trace ===============================

    /home/daveo/src/node/out/Debug/node(v8::base::debug::StackTrace::StackTrace()+0x1d) [0x5561260d9aad]
    /home/daveo/src/node/out/Debug/node(V8_Fatal+0x10f) [0x5561260d5f45]
    /home/daveo/src/node/out/Debug/node(v8::Utils::OpenHandle(v8::Context const*, bool)+0x63) [0x556124e81bed]
    /home/daveo/src/node/out/Debug/node(v8::Context::Global()+0x2d) [0x556124eb1d45]
    /home/daveo/node8-v8-snapshot/node_modules/v8-profiler/build/profiler/v5.7.0/node-v57-linux-x64/profiler.node(nodex::ActivityControlAdapter::ReportProgressValue(int, int)+0x7d) [0x7fed4f0576d1]
    /home/daveo/src/node/out/Debug/node(v8::internal::HeapSnapshotGenerator::ProgressReport(bool)+0x8b) [0x5561259c6551]
    /home/daveo/src/node/out/Debug/node(bool v8::internal::V8HeapExplorer::IterateAndExtractSinglePass<&v8::internal::V8HeapExplorer::ExtractReferencesPass1>()+0x238) [0x5561259c9c76]
    /home/daveo/src/node/out/Debug/node(v8::internal::V8HeapExplorer::IterateAndExtractReferences(v8::internal::SnapshotFiller*)+0xfa) [0x5561259bec8a]
    /home/daveo/src/node/out/Debug/node(v8::internal::HeapSnapshotGenerator::FillReferences()+0x54) [0x5561259c6674]
    /home/daveo/src/node/out/Debug/node(v8::internal::HeapSnapshotGenerator::GenerateSnapshot()+0xff) [0x5561259c6409]
    /home/daveo/src/node/out/Debug/node(v8::internal::HeapProfiler::TakeSnapshot(v8::ActivityControl*, v8::HeapProfiler::ObjectNameResolver*)+0xa0) [0x5561259ae534]
    /home/daveo/src/node/out/Debug/node(v8::HeapProfiler::TakeHeapSnapshot(v8::ActivityControl*, v8::HeapProfiler::ObjectNameResolver*)+0x2b) [0x556124ec7a95]
    /home/daveo/node8-v8-snapshot/node_modules/v8-profiler/build/profiler/v5.7.0/node-v57-linux-x64/profiler.node(nodex::HeapProfiler::TakeSnapshot(Nan::FunctionCallbackInfo<v8::Value> const&)+0x75) [0x7fed4f05701b]
    /home/daveo/node8-v8-snapshot/node_modules/v8-profiler/build/profiler/v5.7.0/node-v57-linux-x64/profiler.node(+0x10616) [0x7fed4f050616]
    /home/daveo/src/node/out/Debug/node(v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&))+0x144) [0x556124ee5d1c]
    /home/daveo/src/node/out/Debug/node(+0x1712b6c) [0x556124fd2b6c]
    /home/daveo/src/node/out/Debug/node(+0x1710b6d) [0x556124fd0b6d]
    /home/daveo/src/node/out/Debug/node(v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*)+0xe5) [0x556124fd0917]
    [0x30d70b904204]
Illegal instruction (core dumped)
@linxuanwei
Copy link

linxuanwei commented Jun 8, 2017

+1 same issue

segmentation fault

@ hustxiaoc

@hyj1991
Copy link

hyj1991 commented Jun 16, 2017

@cxreg @linxuanwei try this:
https://www.npmjs.com/package/v8-profiler-node8

@gergelyke
Copy link

@hyj1991 can you help me what have you changed there in the cc / h files?

@cxreg
Copy link
Author

cxreg commented Jun 16, 2017

It looks like he simply isn't passing the progress callback when running in Node 8

https://github.com/hyj1991/v8-profiler/commit/3b861d181c180036cbf7d998dd587fefd304d9d4#diff-22c7be63c06f678aed1e00c0faeca60a

I can confirm that this is the source of the crash, although I think this will cause tests to fail, and constitutes a reduction in functionality. From what I can tell it looks like Nan::GetCurrentContext() is returning an empty context in Node 8, where this was not the case in earlier versions. I don't know enough about v8 to understand why, though

@gergelyke
Copy link

@rvagg can you help us here please?

@rvagg
Copy link
Contributor

rvagg commented Jun 19, 2017

@kkoopa @bnoordhuis might have a clue here

@bnoordhuis
Copy link
Contributor

ReportProgressValue() calls into JS land. Don't do that. :-)

It may have worked by accident in the past but I don't think it was ever safe to do so.

@kkoopa
Copy link

kkoopa commented Jun 19, 2017

Also, why are the adapter classes heap allocated? They never seem to be freed.

@cxreg
Copy link
Author

cxreg commented Jun 19, 2017

The adapter needs to be heap allocated because Serialize isn't completed synchronously. I think there's a missing delete this; in OutputStreamAdapter::EndOfStream

Edit: nevermind, it does complete before Serialize returns, it just iterates over WriteAsciiChunk before doing so. So yeah this probably should be on the stack

@linxuanwei
Copy link

@hyj1991

"v8-profiler-node8" ( version: 5.7.6) is OK

Can you merge this to v8-profiler ?

@lmyzzu
Copy link

lmyzzu commented Jul 19, 2017

snapshot.delete() segmentation fault core dumped also when using v8-profiler-node8
I'm using node 8.1.4

puzzling

profile1.export(function(error, result) {
fs.writeFileSync('profile1.json', result);
profile1.delete(); //is ok
});

snapshot2.export()
.pipe(fs.createWriteStream('snapshot2.json'))
.on('finish', snapshot2.delete); //segmentation fault

//and It's ok as follow :

snapshot2.export()
.pipe(fs.createWriteStream('snapshot2.json'))
.on('finish', function(){
snapshot2.delete();
});

@hyj1991
Copy link

hyj1991 commented Jul 24, 2017

@lmyzzu
follow will be ok:

snapshot2.export()
.pipe(fs.createWriteStream('snapshot2.json'))
.on('finish', snapshot2.delete.bind(snapshot2));

@cxreg
Copy link
Author

cxreg commented Jul 24, 2017

The delete method should probably throw instead of segfault when called unbound. But that's a separate issue from this one

carlevans719 added a commit to webantic/v8-profiler that referenced this issue Sep 19, 2017
fix: takeSnapshot segfaults on Node 8 (node-inspector#112)
@paulrutter
Copy link

paulrutter commented Oct 11, 2017

Although v8-profiler is great, there doesn't seem to be a lot of activity in the repo. That's why i've created a new module which works from nodejs 6.3+, so nodejs 8 works as well. It can create heapdumps and CPU profiles as well.

See https://www.npmjs.com/package/node-oom-heapdump

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

9 participants