fix(drivers/smb): close smb sessions during transfers - #2579
Conversation
Co-authored-by: Codex <[email protected]> Signed-off-by: shoyuf <[email protected]>
117c61c to
ce34736
Compare
fdc6f0a to
ce34736
Compare
xrgzs
changed the title
- Reuse the existing locked SMB initialization helper. - Remove the obsolete connection-check wrapper. Signed-off-by: shoyuf <[email protected]> Co-authored-by: Codex <[email protected]>
- Merge the latest upstream changes into the SMB lifecycle branch. - Preserve connection lifecycle state while adopting atomic.Int64 timestamps. Signed-off-by: shoyuf <[email protected]> Co-authored-by: Codex <[email protected]>
- Remove the unreferenced singleflight initialization wrapper. - Drop imports used only by the deleted wrapper. Signed-off-by: shoyuf <[email protected]> Co-authored-by: Codex <[email protected]>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba137cf3e0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _ = d.fs.Umount() | ||
| } | ||
| return nil | ||
| return d.closeFS() |
There was a problem hiding this comment.
When the SMB server is already unreachable or the TCP session is half-closed, closeFS() can return errors from Umount, Logoff, or conn.Close; DisableStorage and UpdateStorage treat any Drop error as fatal, so this change can prevent admins from disabling or updating a broken SMB mount even though teardown has already cleared the local SMB handles. The previous implementation ignored Umount errors, matching the best-effort close behavior used by similar network drivers, so Drop should avoid propagating these cleanup failures or only log them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 58999a0. SMB.Drop() now treats teardown as best-effort: it still calls closeFS() to clear local handles but ignores cleanup errors so disable and update operations can continue. Verified with go test ./drivers/smb.
- Ignore SMB teardown errors after clearing local connection handles. - Allow storage disable and update operations to continue after cleanup failures. Signed-off-by: shoyuf <[email protected]> Co-authored-by: Codex <[email protected]>
4 participants
Summary / 摘要
Fix SMB session lifecycle handling during long transfers.
Store the
*smb2.Sessionin the SMB driver so cleanup can callSession.Logoff(), not onlyShare.Umount().Add connection leases around SMB operations so idle refresh does not replace a session while a file stream, upload, or same-storage copy is still active.
Keep the SMB download lease alive until the returned
Linkstream is closed.Log off partially initialized sessions if
Mountfails afterDialsucceeds.This PR has breaking changes.
This PR changes public API, config, storage format, or migration behavior.
This PR requires corresponding changes in related repositories.
Related repository PRs:
Related Issues / 关联 Issue
Fixes #2578
Related to #2228
Testing / 测试
gofmt drivers/smb/driver.go drivers/smb/util.goGOCACHE=/private/tmp/openlist-go-build-cache GOMODCACHE=/private/tmp/openlist-go-mod-cache GOPROXY=https://goproxy.cn,direct go test ./drivers/smbgo test ./...go test ./...was attempted, but unrelated existing packages fail:non-constant format stringdiagnostics;pkg/aria2/rpcexpects a local aria2 RPC server atlocalhost:6800.The SMB package itself compiles successfully.
Checklist / 检查清单
gofmt.AI Disclosure / AI 使用声明
Tools used / 使用工具:
Usage scope / 使用范围:
Code generation / 代码生成
Refactoring / 重构
Documentation / 文档
Tests / 测试
Review assistance / 审查辅助
I have reviewed and validated all AI-assisted content included in this PR.
I have ensured that all AI-assisted commits include
Co-Authored-Byattribution.I can reproduce all AI-assisted content included in this PR without any AI tools.