nodejs / nodejs/nan

nan tests hit DCHECKs on latest node

Open
#858 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
3.4k
Forks
531
Avg merge
21m
Merged PRs (30d)
1

Description

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)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with test/js/nannew-test.js and the corresponding cases in test/cpp/nannew.cpp, then reproduce with the Debug Node build using the commands in the issue. Inspect the reported DCHECK in V8's deps/v8/src/api-inl.h and determine why the failing test reaches it. Done means the nannew test passes under Debug builds as well as Release builds.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, node.js
Domain
testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.