nan tests hit DCHECKs on latest node
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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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