Support for node v12.x

[dhessler]

Hello,

I am running node v12.x, which includes improved async stack traces.
I'm trying to debug a potential memory leak in one of my applications, where taking snapshots is consistently crashing the process, and found this library which looks really promising.

Unfortunately the build is failing on node 12.7.0; I get a little further after naively updating all dependencies to their latest versions:

npm install

> gc-stats@1.4.0 install ******/node-oom-heapdump/node_modules/gc-stats
> node-pre-gyp install --fallback-to-build

node-pre-gyp WARN Using needle for node-pre-gyp https download
node-pre-gyp WARN Tried to download(403): https://node-binaries.s3.amazonaws.com/gcstats/v1.4.0/Release/node-v72-darwin-x64.tar.gz
node-pre-gyp WARN Pre-built binaries not found for gc-stats@1.4.0 and node@12.7.0 (node-v72 ABI, unknown) (falling back to source compile with node-gyp)
  CXX(target) Release/obj.target/gcstats/src/gcstats.o
  SOLINK_MODULE(target) Release/gcstats.node
  COPY ******/node-oom-heapdump/node_modules/gc-stats/build/gcstats/v1.4.0/Release/node-v72-darwin-x64/gcstats.node
  TOUCH Release/obj.target/action_after_build.stamp

> node-oom-heapdump@1.1.4 install ******/node-oom-heapdump
> node-gyp rebuild

  CXX(target) Release/obj.target/node_oom_heapdump_native/lib/node_oom_heapdump_native.o
../lib/node_oom_heapdump_native.cc:77:44: error: no matching member function for call to
      'ToString'
  String::Utf8Value fArg(isolate, args[0]->ToString());
                                  ~~~~~~~~~^~~~~~~~
/Users/david/.node-gyp/12.7.0/include/node/v8.h:2535:44: note: candidate function not
      viable: requires single argument 'context', but no arguments were provided
  V8_WARN_UNUSED_RESULT MaybeLocal<String> ToString(
                                           ^
/Users/david/.node-gyp/12.7.0/include/node/v8.h:2551:31: note: candidate function not
      viable: requires single argument 'isolate', but no arguments were provided
                Local<String> ToString(Isolate* isolate) const);
                              ^
../lib/node_oom_heapdump_native.cc:83:27: error: no matching member function for call to
      'BooleanValue'
  addTimestamp = args[1]->BooleanValue();
                 ~~~~~~~~~^~~~~~~~~~~~
/Users/david/.node-gyp/12.7.0/include/node/v8.h:2566:8: note: candidate function not
      viable: requires single argument 'isolate', but no arguments were provided
  bool BooleanValue(Isolate* isolate) const;
       ^
/Users/david/.node-gyp/12.7.0/include/node/v8.h:2569:51: note: candidate function not
      viable: requires single argument 'context', but no arguments were provided
                V8_WARN_UNUSED_RESULT Maybe<bool> BooleanValue(
                                                  ^
2 errors generated.
make: *** [Release/obj.target/node_oom_heapdump_native/lib/node_oom_heapdump_native.o] Error 1
gyp ERR! build error
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/usr/local/lib/node_modules/npm/node_modules/node-gyp/lib/build.js:266:23)
gyp ERR! stack     at ChildProcess.emit (events.js:203:13)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:272:12)
gyp ERR! System Darwin 18.7.0
gyp ERR! command "/usr/local/bin/node" "/usr/local/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
gyp ERR! cwd ******/node-oom-heapdump
gyp ERR! node -v v12.7.0
gyp ERR! node-gyp -v v3.8.0
gyp ERR! not ok
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! node-oom-heapdump@1.1.4 install: `node-gyp rebuild`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the node-oom-heapdump@1.1.4 install script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

Do you expect to add support for node 12 anytime soon?
Thanks!

[paulrutter]

Hi @dhessler,

Yes, the company i work for is planning to move to Node 12 when it becomes LTS in october.
Part of that upgrade is making sure this module works on 12 as well.

So yes, i expect to add support for Node 12 in the near future.

Regards,

[paulrutter]

I haven't been able to test it thoroughly, but i managed to modify the native code to compile for Node.js 12. I've committed my code in a branch (node12) and published a "beta" version on npm, to test with.

See https://www.npmjs.com/package/node-oom-heapdump/v/1.2.0-beta.0.
Let me know what your experiences are.

UPDATE:

<--- Last few GCs --->

[83:0x3af7320]     1265 ms: Mark-sweep 79.9 (81.3) -> 79.6 (80.8) MB, 59.9 / 0.0 ms  (+ 22.0 ms in 16 steps since start of marking, biggest step 17.4 ms, walltime since start of marking 98 ms) (average mu = 0.460, current mu = 0.169) allocation failure sc[83:0x3af7320]     1358 ms: Mark-sweep 79.8 (80.8) -> 79.4 (80.8) MB, 75.9 / 0.0 ms  (+ 11.8 ms in 2 steps since start of marking, biggest step 11.8 ms, walltime since start of marking 92 ms) (average mu = 0.300, current mu = 0.053) allocation failure sca

<--- JS stacktrace --->
Cannot get stack trace in GC.


#
# Fatal error in , line 0
# unreachable code
#
#
#
#FailureMessage Object: 0x7fe869953880[2019-09-02T13:15:45+01:00]

It looks like i'll need to dig a little deeper; creating a heapdump through the v8 API causes a FATAL error on Node12, where this used to work earlier.

[dhessler]

wow thanks @paulrutter for the quick investigation! definitely happy to beta test when ready.

[paulrutter]

I'm having a hard time getting it to work on Node.js 12.x.
See nodejs/node#27552 (comment).

I think i'll need help from the v8 team in order to proceed.
It seems that the API's haven't changed, but the context in which the API is called no longer seems to work on Node.js 12.

To be continued.

[paulrutter]

@dhessler after investigation, it looks like the combination of node startup flags i was using for my test case, caused the module to be unstable on nodejs 12.
Although this is not as it should be (i'm waiting on response from the nodejs team), it does work when not using those flags.

So, i would suggest we start using it on Node 12, to gather hands on experience with it.
If you encounter any issues, please let me know.

I will start using it in the next couple of months for sure.
Version 1.2.0 is now published to npm :)

[paulrutter]

I'm closing the issue for now.
Let me know when you encounter any issues by filing a new issue.

[jorisw]

@paulrutter Which were the flags causing Node 12 to run badly? I'm having the same problem with v2.0.0 on Node 12.10.

[paulrutter]

See fac8e92
These were:

• --optimize_for_size
• --always_compact