Revert "deps: update libuv to 1.52.1"#62497
Conversation
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.
|
Review requested:
|
|
Note that i was able to repro the issue on windows locally; and after applying the revert locally it has not reoccurred yet. |
|
cc @nodejs/libuv |
Codecov Report✅ All modified and coverable lines are covered by tests. 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 🚀 New features to boost your workflow:
|
|
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. |
|
10 runs is probably too small a sample to be confident. Are you using the |
|
I can reproduce locally |
|
Fast-track has been requested by @panva. Please 👍 to approve. |
|
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. |
|
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 |
|
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. |
|
With the case above, the issue starts with libuv/libuv@3a9a6e3 |
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.