nan tests hit DCHECKs on latest node

[MarshallOfSound]

I discovered this while running the nan test suite against Electron on our CI (we run tests with DCHECKs enabled). Just to cover my bases I ran the same nan tests against a Debug build of node and the same DCHECKs were hit.

#
# Fatal error in ../deps/v8/src/api-inl.h, line 126
# Debug check failed: allow_empty_handle || that != nullptr.
#
#
#
#FailureMessage Object: 0x7ffeefbfc2a0not ok test/js/nannew-test.js ......................... 22/23
    Command: "/Users/sattard/projects/nodejs/node/out/Debug/node nannew-test.js"
    TAP version 13
    ok 1 ../cpp/nannew.cpp:61: New<Array>()->Length() == 0
    ok 2 ../cpp/nannew.cpp:62: New<Array>(7)->Length() == 7
    ok 3 ../cpp/nannew.cpp:63: assertType<Array>(New<Array>(7))
    ok 4 ../cpp/nannew.cpp:73: New<Boolean>(true)->Value() == true
    ok 5 ../cpp/nannew.cpp:74: New<Boolean>(false)->Value() == false
    ok 6 ../cpp/nannew.cpp:75: assertType<Boolean>( New<Boolean>(true))
    ok 7 ../cpp/nannew.cpp:77: New(true)->Value() == true
    ok 8 ../cpp/nannew.cpp:78: New(false)->Value() == false
    ok 9 ../cpp/nannew.cpp:79: assertType<Boolean>( New(true))
    ok 10 ../cpp/nannew.cpp:94: assertType<BooleanObject>( New<BooleanObject>(true))
    ok 11 ../cpp/nannew.cpp:95: New<BooleanObject>(true)->ValueOf() == true
    ok 12 ../cpp/nannew.cpp:96: New<BooleanObject>(false)->ValueOf() == false
    ok 13 ../cpp/nannew.cpp:106: assertType<Context>( New<Context>())
    ok 14 ../cpp/nannew.cpp:108: assertType<Context>( New<Context>(&extensions))
    ok 15 ../cpp/nannew.cpp:111: assertType<Context>( New<Context>(static_cast<ExtensionConfiguration *>(__null) , Local<ObjectTemplate>()))
    ok 16 ../cpp/nannew.cpp:113: assertType<Context>( New<Context>(&extensions, Local<ObjectTemplate>()))
    ok 17 ../cpp/nannew.cpp:116: assertType<Context>( New<Context>(&extensions , Local<ObjectTemplate>(), Local<Value>()))
    ok 18 ../cpp/nannew.cpp:127: assertType<Date>( New<Date>(static_cast<double>(time(__null))).ToLocalChecked())
    ok 19 ../cpp/nannew.cpp:139: New<External>(&ttt)->Value() == &ttt
    ok 20 ../cpp/nannew.cpp:140: assertType<External>(New<External>(&ttt))
    ok 21 ../cpp/nannew.cpp:149: assertType<Function>(New<Function>(testFunction))
    ok 22 ../cpp/nannew.cpp:151: assertType<Function>(New<Function>(testFunction, data))
    not ok 23 test/js/nannew-test.js
      ---
        exit:    ~
        signal:  SIGILL
        stderr:  |
          #
          # Fatal error in ../deps/v8/src/api-inl.h, line 126
          # Debug check failed: allow_empty_handle || that != nullptr.
          #
          #
          #
          #FailureMessage Object: 0x7ffeefbfc2a0
        command: "/Users/sattard/projects/nodejs/node/out/Debug/node nannew-test.js"
      ...

    1..23
    # tests 23
    # pass  22
    # fail  1

The same tests pass in Release builds (as DCHECKs are disabled) but I think some time should be spent figuring out what is going wrong here as V8 clearly isn't happy with what is happening 🤔

To reproduce with a local build of node with a directory structure of

nodejs
  / node
  / nan

# Build node + headers
cd nodejs/node
./configure --debug
make -j4
make tar-headers
tar -xzf node-v13.0.0-headers.tar.gz

cd ../nan
# Build nan test modules with right headers
npm_config_nodedir=$(pwd)/../node/node-v13.0.0 npx node-gyp rebuild --directory test
# Run nan tests with built version of node
../node/out/Debug/node node_modules/.bin/tap test/js/*-test.js

Note that although the example above uses HEAD of node (v13) I have reproed with v12 as well (which is what Electron currently ships)

[kkoopa]

Based on the progression of test cases, I would guess it is https://github.com/nodejs/nan/blob/master/test/cpp/nannew.cpp#L161 that triggers it, but I do not see why.

[kkoopa]

Might no logger be allowed to create empty handles, at least not of FunctionTemplate. Utils::OpenHandle defaults to that.