takeSnapshot segfaults on Node 8

[cxreg]

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]

+1 same issue

segmentation fault

@ hustxiaoc

[hyj1991]

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

[gergelyke]

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

[cxreg]

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]

@rvagg can you help us here please?

[rvagg]

@kkoopa @bnoordhuis might have a clue here

[bnoordhuis]

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]

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

[cxreg]

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]

@hyj1991

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

Can you merge this to v8-profiler ?

[lmyzzu]

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]

@lmyzzu
follow will be ok:

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

[cxreg]

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

[paulrutter]

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