src: fix libuv assertion on windows#61999
Conversation
Set the pointer to `nullptr` after calling `uv_close` to avoid assertions. Fixes: nodejs#56645
53bbcc6 to
b721fef
Compare
|
That handle is gonna leak, and I don't think it's gonna fix anything. This looks pretty much a AI-generated solution... that does actually nothing regarding your intention. 💔 |
|
This is not an AI-generated solution, it's an inspiration I got from other code. Lines 411 to 436 in a8eb690 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #61999 +/- ##
==========================================
- Coverage 89.77% 89.73% -0.05%
==========================================
Files 674 676 +2
Lines 205705 206068 +363
Branches 39449 39514 +65
==========================================
+ Hits 184670 184907 +237
- Misses 13280 13312 +32
- Partials 7755 7849 +94
🚀 New features to boost your workflow:
|
My bad... but sadly still... I don't think this is gonna fix the fundamental problem at all and still leaking. |
src/node_platform.cc
Outdated
| double delay_in_seconds) { | ||
| auto locked = tasks_.Lock(); | ||
|
|
||
| if (flush_tasks_ == nullptr) return; |
There was a problem hiding this comment.
If "just not sending" is indeed a valid solution to this issue (which I have not verified), you could set a boolean flag next to flush_tasks_ instead of making it a heap-allocated variable
There was a problem hiding this comment.
Thanks!
v8 is calling PostDelayedTaskOnWorkerThread here, which looks like a task for wasm caching.
https://github.com/v8/v8/blob/1fb9a7b9671a724ebdcf57db3807d4af56dc6cbb/src/wasm/module-compiler.cc#L4004-L4011
Ignoring it should be safe, because anyway, the delay here is 2 seconds, and the node will have finished long before then.
|
@juanarbol I understand now. The AI is telling me that the handle wasn't deleted. AI is more familiar with C++ than I am. 🤦♂️ |
Set the pointer to
nullptrafter callinguv_closeto avoid assertions.Fixes: #56645