mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 05:06:59 +00:00
fix(config): delete read-only thread directories on Windows (#7446)
`Paths.delete_thread_dir()` called `shutil.rmtree()` directly. Windows
refuses to unlink a file that carries the read-only attribute
(`WinError 5`) where POSIX only needs a writable parent, so one read-only
file under `user-data/` made the whole thread undeletable: `DELETE
/api/threads/{id}` answered 500 `Failed to delete local thread data`,
`rmtree` had already removed everything it walked before that entry, and
every later retry failed on the same file.
Retry at the call site: clear the read-only attribute tree-wide and call
`rmtree()` once more. Only `PermissionError` is retried, the retry still
raises when it cannot help, so a file another process holds open stays a
real error instead of becoming a silent skip. A `onexc` handler cannot do
this portably: POSIX `_rmtree_safe_fd()` catches the handler's own
`PermissionError`, re-dispatches it as a `scandir` error and leaves the
deletion to fail later as `ENOTEMPTY`.
Co-authored-by: Aling33 <along308@users.noreply.github.com>
This commit is contained in:
parent
afcb915de3
commit
cb6ee72132
@ -3,6 +3,7 @@ import logging
|
||||
import os
|
||||
import re
|
||||
import shutil
|
||||
import stat
|
||||
from pathlib import Path, PureWindowsPath
|
||||
|
||||
from deerflow.config.runtime_paths import runtime_home
|
||||
@ -30,6 +31,25 @@ def _validate_thread_id(thread_id: str) -> str:
|
||||
return validate_thread_id(thread_id)
|
||||
|
||||
|
||||
def _clear_read_only_tree(root: Path) -> None:
|
||||
"""Clear the read-only attribute under *root* before repeating a failed deletion.
|
||||
|
||||
Windows refuses ``unlink``/``rmdir`` on a read-only file or directory, where
|
||||
POSIX needs only a writable parent, so a restored archive or a mode-preserving
|
||||
sync leaves a thread undeletable (``WinError 5``). Best effort: entries that
|
||||
vanished or cannot be stat-ed are skipped, and the retry reports what is left.
|
||||
Symlinks are skipped because ``chmod`` would follow them to their target.
|
||||
"""
|
||||
for entry in (root, *root.rglob("*")):
|
||||
try:
|
||||
mode = entry.lstat().st_mode
|
||||
if stat.S_ISLNK(mode) or mode & stat.S_IWRITE:
|
||||
continue
|
||||
entry.chmod(mode | stat.S_IWRITE)
|
||||
except OSError:
|
||||
continue
|
||||
|
||||
|
||||
def _validate_user_id(user_id: str) -> str:
|
||||
"""Validate a user ID before using it in filesystem paths."""
|
||||
if not _SAFE_USER_ID_RE.fullmatch(user_id):
|
||||
@ -488,11 +508,22 @@ class Paths:
|
||||
def delete_thread_dir(self, thread_id: str, *, user_id: str | None = None) -> None:
|
||||
"""Delete all persisted data for a thread.
|
||||
|
||||
The operation is idempotent: missing thread directories are ignored.
|
||||
Idempotent: missing thread directories are ignored. A file the host marks
|
||||
read-only is retried once after clearing that bit, because Windows refuses
|
||||
to unlink it where POSIX only needs a writable parent; a file that stays
|
||||
undeletable still raises.
|
||||
"""
|
||||
thread_dir = self.thread_dir(thread_id, user_id=user_id)
|
||||
if thread_dir.exists():
|
||||
try:
|
||||
shutil.rmtree(thread_dir)
|
||||
except FileNotFoundError:
|
||||
return
|
||||
except PermissionError:
|
||||
_clear_read_only_tree(thread_dir)
|
||||
try:
|
||||
shutil.rmtree(thread_dir)
|
||||
except FileNotFoundError:
|
||||
return
|
||||
|
||||
def resolve_virtual_path(self, thread_id: str, virtual_path: str, *, user_id: str | None = None) -> Path:
|
||||
"""Resolve a sandbox virtual path to the actual host filesystem path.
|
||||
|
||||
@ -1,5 +1,7 @@
|
||||
"""Tests for user-scoped path resolution in Paths."""
|
||||
|
||||
import os
|
||||
import stat
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
@ -265,6 +267,46 @@ class TestEnsureAndDeleteWithUserId:
|
||||
def test_delete_thread_dir_idempotent(self, paths: Paths):
|
||||
paths.delete_thread_dir("nonexistent", user_id="u1") # should not raise
|
||||
|
||||
def test_delete_thread_dir_retries_read_only_files(self, paths: Paths, monkeypatch):
|
||||
"""Windows refuses a read-only file even though the parent is writable."""
|
||||
paths.ensure_thread_dirs("t1", user_id="u1")
|
||||
artifact = paths.sandbox_outputs_dir("t1", user_id="u1") / "report.pdf"
|
||||
artifact.write_text("quarterly")
|
||||
artifact.chmod(0o444)
|
||||
real_unlink = os.unlink
|
||||
|
||||
def windows_unlink(path, *args, **kwargs):
|
||||
# POSIX deletes through ``os.unlink(name, dir_fd=...)``, so the refusal
|
||||
# has to key on that call shape to fire anywhere but Windows.
|
||||
dir_fd = kwargs.get("dir_fd")
|
||||
try:
|
||||
if dir_fd is not None:
|
||||
mode = os.stat(path, dir_fd=dir_fd, follow_symlinks=False).st_mode
|
||||
else:
|
||||
mode = os.stat(path).st_mode
|
||||
except FileNotFoundError:
|
||||
return real_unlink(path, *args, **kwargs)
|
||||
if stat.S_ISREG(mode) and not mode & stat.S_IWRITE:
|
||||
raise PermissionError(13, "Access is denied", str(path))
|
||||
return real_unlink(path, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(os, "unlink", windows_unlink)
|
||||
paths.delete_thread_dir("t1", user_id="u1")
|
||||
assert not paths.thread_dir("t1", user_id="u1").exists()
|
||||
|
||||
def test_delete_thread_dir_keeps_raising_when_the_file_stays_locked(self, paths: Paths, monkeypatch):
|
||||
"""A file still held open is a real failure; the retry must not swallow it."""
|
||||
paths.ensure_thread_dirs("t1", user_id="u1")
|
||||
(paths.sandbox_uploads_dir("t1", user_id="u1") / "notes.txt").write_text("x")
|
||||
|
||||
def locked_unlink(path, *args, **kwargs):
|
||||
raise PermissionError(13, "The process cannot access the file", str(path))
|
||||
|
||||
monkeypatch.setattr(os, "unlink", locked_unlink)
|
||||
with pytest.raises(PermissionError):
|
||||
paths.delete_thread_dir("t1", user_id="u1")
|
||||
assert paths.thread_dir("t1", user_id="u1").exists()
|
||||
|
||||
def test_ensure_thread_dirs_legacy_still_works(self, paths: Paths):
|
||||
paths.ensure_thread_dirs("t1")
|
||||
assert paths.sandbox_work_dir("t1").is_dir()
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user