Skip to content

Conversation

@anonrig
Copy link
Member

@anonrig anonrig commented Nov 18, 2024

Adds a V8 fast api test for binding.getLibuvNow() function.

cc @nodejs/cpp-reviewers

@anonrig anonrig requested a review from targos November 18, 2024 15:38
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. labels Nov 18, 2024
@anonrig
Copy link
Member Author

anonrig commented Nov 18, 2024

@targos I'm getting a runtime error with this. Am I missing something?

NOTE: The test started as a child_process using these flags: [ '--expose-internals', '--no-warnings', '--allow-natives-syntax' ] Use NODE_SKIP_FLAG_CHECK to run the test with the original flags.


#
# Fatal error in , line 0
# Check failed: v8_flags.fuzzing.
#
#
#
#FailureMessage Object: 0x16d012088
----- Native stack trace -----

 1: 0x102f976b4 node::NodePlatform::GetStackTracePrinter()::$_0::__invoke() [/Users/yagiz/coding/node/out/Release/node]
 2: 0x1045f5f9c V8_Fatal(char const*, ...) [/Users/yagiz/coding/node/out/Release/node]
 3: 0x1037742e8 v8::internal::Runtime_OptimizeOsr(int, unsigned long*, v8::internal::Isolate*) [/Users/yagiz/coding/node/out/Release/node]
 4: 0x103de3848 Builtins_CEntry_Return1_ArgvInRegister_NoBuiltinExit [/Users/yagiz/coding/node/out/Release/node]
 5: 0x103ec6114 Builtins_CallRuntimeHandler [/Users/yagiz/coding/node/out/Release/node]
 6: 0x103d4c838 Builtins_InterpreterEntryTrampoline [/Users/yagiz/coding/node/out/Release/node]
 7: 0x103d4c838 Builtins_InterpreterEntryTrampoline [/Users/yagiz/coding/node/out/Release/node]
 8: 0x103d4c838 Builtins_InterpreterEntryTrampoline [/Users/yagiz/coding/node/out/Release/node]
 9: 0x103d4c838 Builtins_InterpreterEntryTrampoline [/Users/yagiz/coding/node/out/Release/node]
10: 0x103d4c838 Builtins_InterpreterEntryTrampoline [/Users/yagiz/coding/node/out/Release/node]
11: 0x103d4c838 Builtins_InterpreterEntryTrampoline [/Users/yagiz/coding/node/out/Release/node]
12: 0x103d4c838 Builtins_InterpreterEntryTrampoline [/Users/yagiz/coding/node/out/Release/node]
13: 0x103d4c838 Builtins_InterpreterEntryTrampoline [/Users/yagiz/coding/node/out/Release/node]
14: 0x103d4c838 Builtins_InterpreterEntryTrampoline [/Users/yagiz/coding/node/out/Release/node]
15: 0x103d4c838 Builtins_InterpreterEntryTrampoline [/Users/yagiz/coding/node/out/Release/node]
16: 0x103d4a50c Builtins_JSEntryTrampoline [/Users/yagiz/coding/node/out/Release/node]
17: 0x103d4a1b0 Builtins_JSEntry [/Users/yagiz/coding/node/out/Release/node]
18: 0x1032a6980 v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) [/Users/yagiz/coding/node/out/Release/node]
19: 0x1032a62e0 v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*) [/Users/yagiz/coding/node/out/Release/node]
20: 0x103142758 v8::Function::Call(v8::Isolate*, v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) [/Users/yagiz/coding/node/out/Release/node]
21: 0x102ef65ac node::builtins::BuiltinLoader::CompileAndCall(v8::Local<v8::Context>, char const*, node::Realm*) [/Users/yagiz/coding/node/out/Release/node]
22: 0x102fa805c node::Realm::ExecuteBootstrapper(char const*) [/Users/yagiz/coding/node/out/Release/node]
23: 0x102ed513c node::StartExecution(node::Environment*, char const*) [/Users/yagiz/coding/node/out/Release/node]
24: 0x102ed5090 node::StartExecution(node::Environment*, std::__1::function<v8::MaybeLocal<v8::Value> (node::StartExecutionCallbackInfo const&)>) [/Users/yagiz/coding/node/out/Release/node]
25: 0x102e2ac7c node::LoadEnvironment(node::Environment*, std::__1::function<v8::MaybeLocal<v8::Value> (node::StartExecutionCallbackInfo const&)>, std::__1::function<void (node::Environment*, v8::Local<v8::Value>, v8::Local<v8::Value>)>) [/Users/yagiz/coding/node/out/Release/node]
26: 0x102f63840 node::NodeMainInstance::Run() [/Users/yagiz/coding/node/out/Release/node]
27: 0x102ed81b4 node::Start(int, char**) [/Users/yagiz/coding/node/out/Release/node]
28: 0x1886c8274 start [/usr/lib/dyld]
[1]    23707 trace trap  out/Release/node test/parallel/test-timers-now.js

@codecov
Copy link

codecov bot commented Nov 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.96%. Comparing base (a2edde4) to head (2081338).
Report is 831 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #55905      +/-   ##
==========================================
- Coverage   88.49%   87.96%   -0.54%     
==========================================
  Files         653      656       +3     
  Lines      187735   188391     +656     
  Branches    36181    35976     -205     
==========================================
- Hits       166141   165710     -431     
- Misses      14819    15846    +1027     
- Partials     6775     6835      +60     
Files with missing lines Coverage Δ
src/timers.cc 86.72% <ø> (ø)

... and 116 files with indirect coverage changes

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@anonrig anonrig requested a review from joyeecheung November 19, 2024 04:55
@targos
Copy link
Member

targos commented Nov 19, 2024

I'm getting a runtime error with this. Am I missing something?

I think it's because you can't directly optimize a native function. You need to wrap it in a JS function and optimize that.

@anonrig anonrig force-pushed the add-fast-api-tests-timers branch 2 times, most recently from 4b7036b to d30d54f Compare November 27, 2024 23:22
@nodejs-github-bot
Copy link
Collaborator

@anonrig
Copy link
Member Author

anonrig commented Nov 27, 2024

I'm getting a runtime error with this. Am I missing something?

I think it's because you can't directly optimize a native function. You need to wrap it in a JS function and optimize that.

Thanks. I updated the pull-request and you're indeed correct. It fixed the crash.

@anonrig anonrig force-pushed the add-fast-api-tests-timers branch from d30d54f to 2081338 Compare November 28, 2024 17:32
@nodejs-github-bot
Copy link
Collaborator

@anonrig anonrig requested a review from jasnell November 28, 2024 18:30
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@juanarbol juanarbol added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 5, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 5, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@anonrig
Copy link
Member Author

anonrig commented Dec 6, 2024

@targos any idea why this test is failing on arm-debug?

@targos
Copy link
Member

targos commented Dec 6, 2024

It means that the fast API is not called during the test. Have you not tried it locally with debug mode ?

@anonrig anonrig closed this Mar 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants