Skip to content

Revert "deps: update libuv to 1.52.1"#62497

Closed
jasnell wants to merge 1 commit intonodejs:mainfrom
jasnell:jasnell/revert-5bebd7e
Closed

Revert "deps: update libuv to 1.52.1"#62497
jasnell wants to merge 1 commit intonodejs:mainfrom
jasnell:jasnell/revert-5bebd7e

Conversation

@jasnell
Copy link
Copy Markdown
Member

@jasnell jasnell commented Mar 29, 2026

This reverts commit 5bebd7e.

The update appears to be causing persistent flakiness in test-http(s), test-net-, and test-tls- tests on Windows. The flakiness manifests as an abort apparently caused by a race condition introduced in connection cleanup on process teardown. Root cause still needs to be fully determined. Reverting the update will hopefully get us back to a stable state while the underlying issue is investigated and resolved.

This reverts commit 5bebd7e.

The update appears to be causing persistent flakiness in test-http(s),
test-net-*, and test-tls-* tests on Windows. The flakiness manifests
as an abort apparently caused by a race condition introduced in
connection cleanup on process teardown. Root cause still needs to be
fully determined. Reverting the update will hopefully get us back to
a stable state while the underlying issue is investigated and resolved.
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/security-wg

@nodejs-github-bot nodejs-github-bot added libuv Issues and PRs related to the libuv dependency or the uv binding. needs-ci PRs that need a full CI run. labels Mar 29, 2026
@jasnell
Copy link
Copy Markdown
Member Author

jasnell commented Mar 29, 2026

Note that i was able to repro the issue on windows locally; and after applying the revert locally it has not reoccurred yet.

Copy link
Copy Markdown
Member

@gurgunday gurgunday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@anonrig
Copy link
Copy Markdown
Member

anonrig commented Mar 29, 2026

cc @nodejs/libuv

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.71%. Comparing base (e78ccd8) to head (8d72d50).
⚠️ Report is 18 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main   #62497    +/-   ##
========================================
  Coverage   89.70%   89.71%            
========================================
  Files         691      692     +1     
  Lines      213935   214039   +104     
  Branches    41050    41065    +15     
========================================
+ Hits       191907   192015   +108     
+ Misses      14095    14092     -3     
+ Partials     7933     7932     -1     

see 53 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.

@lpinca
Copy link
Copy Markdown
Member

lpinca commented Mar 29, 2026

@jasnell I see the issue on CI runs for #62165 and #62158 and that was one week before the libuv update landed, so I don't think the regression is in libuv. I'll try to run git-bisect if I find a way to reproduce, a Windows machine, and time.

@jasnell
Copy link
Copy Markdown
Member Author

jasnell commented Mar 29, 2026

Hmmm.. weird. I haven't been able to reproduce locally on my machine here following the revert.. but then again, this is a really fast machine. If it's a race condition, I have to wonder if system performance has an impact. It failed about 1 out of 10 runs before the revert and 0 out of 10 after the revert. When i'm able to get back to it I'll run the tests longer to see if it shows up without the revert.

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

@aduh95
Copy link
Copy Markdown
Contributor

aduh95 commented Mar 30, 2026

10 runs is probably too small a sample to be confident. Are you using the --repeat flag? python tools/test.py -t 9 --repeat 999 'test/parallel/test-http*' 'test/parallel/test-net-*' 'test/parallel/test-tls-*'

@lpinca
Copy link
Copy Markdown
Member

lpinca commented Mar 30, 2026

I can reproduce locally

~/node ((2e5731eeebe...))
$ python ./tools/test.py --repeat=3000 parallel/test-net-server-keepalive
[06:19|% 100|+ 3000|-   0]: Done

All tests passed.
~/node ((5bebd7eaea2...))
$ python ./tools/test.py --repeat=3000 parallel/test-net-server-keepalive
=== release test-net-server-keepalive ===
Path: parallel/test-net-server-keepalive
Command: C:\Users\lpinca\node\out\Release\node.exe C:\Users\lpinca\node\test\parallel\test-net-server-keepalive.js


...


=== release test-net-server-keepalive ===
Path: parallel/test-net-server-keepalive
Command: C:\Users\lpinca\node\out\Release\node.exe C:\Users\lpinca\node\test\parallel\test-net-server-keepalive.js


[06:48|% 100|+ 2992|-   8]: Done

Failed tests:
C:\Users\lpinca\node\out\Release\node.exe C:\Users\lpinca\node\test\parallel\test-net-server-keepalive.js

...

C:\Users\lpinca\node\out\Release\node.exe C:\Users\lpinca\node\test\parallel\test-net-server-keepalive.js

@aduh95 aduh95 added dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. dont-land-on-v24.x PRs that should not land on the v24.x-staging branch and should not be released in v24.x. dont-land-on-v25.x PRs that should not land on the v25.x-staging branch and should not be released in v25.x. labels Mar 30, 2026
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

@panva panva added the fast-track PRs that do not need to wait for 72 hours to land. label Mar 30, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Fast-track has been requested by @panva. Please 👍 to approve.

@jasnell
Copy link
Copy Markdown
Member Author

jasnell commented Mar 30, 2026

Don't merge yet. I ran tests last night and it still failed with the revert but at a much much lower rate. @mcollina is working on bisecting.

@aduh95, I didn't say I only ran it 10 times, I said it was failing 1 out of every 10 times. I haven't looked at the updated rate yet I only just saw that it still had some crashes. Need to test a bit more and I want to see what Matteo's bisect comes up with.

@aduh95
Copy link
Copy Markdown
Contributor

aduh95 commented Mar 30, 2026

Maybe that's what you meant, but not what you said x) it's interesting you found a 10% failure rate when Luigi found 0.3% in #62497 (comment); in any case, I think it's fine to land ASAP given the current state of CI

@jasnell
Copy link
Copy Markdown
Member Author

jasnell commented Mar 30, 2026

What I've seen locally is the the rate appears to be rather variable. I tried it on a second windows machine with a much slower processor and the rate was significantly different... really just bolsters the case for it being some kind of race condition somewhere. In any case, let's not be in a rush to land this. I want to see what @mcollina's bisect turns up.

@Renegade334 Renegade334 removed the fast-track PRs that do not need to wait for 72 hours to land. label Mar 30, 2026
@lpinca
Copy link
Copy Markdown
Member

lpinca commented Mar 30, 2026

With the case above, the issue starts with libuv/libuv@3a9a6e3

@juanarbol
Copy link
Copy Markdown
Member

@jasnell can we close this in favor of #62511?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. dont-land-on-v24.x PRs that should not land on the v24.x-staging branch and should not be released in v24.x. dont-land-on-v25.x PRs that should not land on the v25.x-staging branch and should not be released in v25.x. libuv Issues and PRs related to the libuv dependency or the uv binding. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants