fs: move FileHandle close on GC to EOL
Automatically closing a FileHandle on garbage collection has been deprecated for some time now. We've said that it will eventually be removed and become and error.
~~This PR also adds a new autoClose option to the FileHandle readableWebStream() API in order to deal with a possible bug in which the readable stream that wraps the FileHandle is passed out of a context where the FileHandle is accessible to be closed.~~ This was extracted and landed in https://github.com/nodejs/node/pull/58548
Codecov Report
Attention: Patch coverage is 40.00000% with 6 lines in your changes missing coverage. Please review.
Project coverage is 90.10%. Comparing base (
ce546e4) to head (37282ce). Report is 34 commits behind head on main.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| src/node_file.cc | 40.00% | 4 Missing and 2 partials :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #58536 +/- ##
==========================================
- Coverage 90.13% 90.10% -0.04%
==========================================
Files 640 640
Lines 188294 188282 -12
Branches 36930 36924 -6
==========================================
- Hits 169721 169651 -70
- Misses 11290 11353 +63
+ Partials 7283 7278 -5
| Files with missing lines | Coverage Δ | |
|---|---|---|
| src/env-inl.h | 94.20% <ø> (-0.07%) |
:arrow_down: |
| src/env.h | 98.14% <ø> (ø) |
|
| src/node_file.cc | 75.79% <40.00%> (-0.07%) |
:arrow_down: |
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
This PR also adds a new
autoCloseoption to theFileHandlereadableWebStream()API in order to deal with a possible bug in which the readable stream that wraps theFileHandleis passed out of a context where theFileHandleis accessible to be closed.
Unless there already is a trivial userspace solution to this or autoClose defaults to true, I think we should land the new option first and wait some extra time before making this EOL (assuming greenish enough CITGM).
CI: https://ci.nodejs.org/job/node-test-pull-request/67213/
CI: https://ci.nodejs.org/job/node-test-pull-request/67440/
~~CI: https://ci.nodejs.org/job/node-test-pull-request/67445/~~ Duplicate... there was already a CI run going CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3612/
CI: https://ci.nodejs.org/job/node-test-pull-request/67467/
CI: https://ci.nodejs.org/job/node-test-pull-request/67574/
There are relevant failures in CI ...
CI: https://ci.nodejs.org/job/node-test-pull-request/67578/
CI: https://ci.nodejs.org/job/node-test-pull-request/67580/
CI: https://ci.nodejs.org/job/node-test-pull-request/67581/
CI: https://ci.nodejs.org/job/node-test-pull-request/67600/
CI: https://ci.nodejs.org/job/node-test-pull-request/67631/
Landed in 2bb7667475e9