Repository navigation
fix(services/fs): avoid initializing unused writers on abort - #8429
Merged
Merged
Conversation
Xuanwo
approved these changes
Oct 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Closes #8428.
Rationale for this change
Aborting a lazy Fs writer currently initializes its inner writer. Cancelling before the first filesystem write can therefore truncate an existing file or create a target and its parents. An unused conditional writer can also fail with
ConditionNotMatcheven though there is nothing to cancel.What changes are included in this PR?
Inspect
FsLazyWriter.innerdirectly during abort: delegate to an existing writer, or return success when it has not been initialized. Keep initialization inwriteandclose. This preserves the existingUnsupportedresult when aborting an initialized non-atomic writer and makes no rollback promise after writes.Add seven tests against real temporary filesystem roots, covering untouched and buffered existing files, missing target directories, conditional writes, atomic target directories, empty close, and initialized non-atomic abort.
Validation
Base
19ba33da9e46738fd4cad29c7ebfd5f3e5ed2cfc: 5 expected regression failures and 2 passing controls. Fix546abff605152e5baa36bf6212fa33514c4b0219:atomic_write_dir.cargo clippy --locked -p opendal-service-fs --all-targets --all-features -- -D warningspassed.python3 scripts/workspace.py cargo fmt -- --checkpassed.Linux, Rust/Cargo 1.99.1. This verifies the affected filesystem service; no full workspace build, other storage service, or cross-platform CI result is claimed.
Follow-up
9058aa2c56d2f77dbf6310efb9d3559d97b1f641normalizes four spaces in the test file's Apache license header to match the repository checker. It changes no executable code (git diff --ignore-all-spaceis empty). HawkEye 7.2.0, the Fs service tests (8 unit, 7 regression, 1 doctest passed; 1 ignored), and core formatting passed on this final commit.Are there any user-facing changes?
Aborting an uninitialized Fs writer preserves existing file contents and avoids creating the target or its parents. Closing an unused writer and aborting an initialized non-atomic writer retain their existing behavior.
Breaking changes
AI Usage Statement
gpt-6.1-sol.CI follow-up (2026-10-26)
The remaining failure is Bindings Lua CI / test:
luarocks makerequests the absentlua51feature. The October 22 main-branch run, at39f6bc7d91534b17e1c0182cb556aeab6cfd6742, fails with the same message before the Lua tests.bindings/lua/and.github/workflows/ci_bindings_lua.ymlare unchanged between that main commit and this PR head, and between the current PR base and head. This establishes a pre-existing Lua build configuration failure; no Lua change has been folded into the Fs fix. The failing check remains outstanding, and no full-CI pass or exemption is claimed. Codex assisted this source/log comparison; no new build was run for this diagnosis.