Windows CI、per-fd 定位 IO 锁与沙箱门面保真

.github/workflows/ci.yml

@@ -29,9 +29,18 @@ jobs:
 
   no-dependencies:
     # The zero-dependency claim is a promise to users, so it gets its own job: install
-    # the package alone and prove the core still imports and passes.
-    runs-on: ubuntu-latest
-    timeout-minutes: 10
+    # the package alone and prove the core still imports and passes.  The matrix is
+    # where Windows gets covered (#7): every platform primitive the package touches
+    # lives in pyvdisk/compat.py, and the emulation tests force those paths on Linux,
+    # but only a real Windows runner exercises msvcrt.locking, the directory-fsync
+    # no-op and drive-letter paths against the actual OS.  PR #5 proved the port
+    # works; this job keeps proving it on every push.
+    runs-on: ${{ matrix.os }}
+    timeout-minutes: 15
+    strategy:
+      fail-fast: false
+      matrix:
+        os: [ubuntu-latest, windows-latest]
     steps:
       - uses: actions/checkout@v4
       - uses: actions/setup-python@v5
@@ -47,3 +56,5 @@ jobs:
           python -c "import pyvdisk; print('pyvdisk', pyvdisk.__version__, 'imported with the standard library only')"
       - name: Tests (accelerator-dependent cases skip)
         run: python -m pytest -q --no-header -p no:cacheprovider
+      - name: Quickstart smoke (stdlib only, end to end)
+        run: python examples/quickstart.py

.github/workflows/pylint.yml

@@ -26,3 +26,24 @@ jobs:
       # declared 3.9 floor still parses is proven by the CI test matrix, not here.
       - name: Lint (errors only)
         run: python -m pylint -E --score=n pyvdisk tests
+
+  errors-windows:
+    # Same errors-only gate on Windows (#7).  pylint -E resolves imports against
+    # the running interpreter, so an unguarded `import fcntl` -- the exact bug
+    # that made PR #5 necessary -- becomes an import-error here instead of a
+    # runtime surprise on a user's machine.  The msvcrt half of compat.py is
+    # likewise only *real* on Windows, so attribute checks see it bound.
+    runs-on: windows-latest
+    timeout-minutes: 15
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
+        with:
+          python-version: "3.12"
+      - name: Install
+        run: |
+          python -m pip install --upgrade pip
+          python -m pip install -e .[dev]
+          python -m pip install "pylint==4.0.9"
+      - name: Lint (errors only)
+        run: python -m pylint -E --score=n pyvdisk tests

pyvdisk/compat.py

@@ -16,8 +16,12 @@
 Why a lock on the emulated positional IO: ``os.pread``/``os.pwrite`` are
 atomic, ``lseek`` + ``read`` is not.  Two threads sharing one handle could
 interleave and read the wrong block, so the emulation serialises through a
-module-level lock.  That costs throughput on Windows only, and only while a
-block IO is in flight; it is not a global lock over the whole disk.
+**per-descriptor** lock: one handle queues behind itself, different handles
+-- and therefore different disks -- never wait on each other.  Stale entries
+after a descriptor is closed are harmless: an fd number reused by a new file
+at worst shares a lock with its predecessor and picks up spurious
+contention, never incorrectness.  That costs throughput on Windows only, and
+only while a block IO on the *same* handle is in flight.
 
 The Windows byte-range lock covers ``[0, LOCK_BYTES)`` -- the same "whole file"
 scope ``flock`` gives us, because every handle in this package locks from
@@ -62,7 +66,23 @@
 #: ``pread``/``pwrite``.
 _HAS_POSITIONAL = hasattr(os, "pread") and hasattr(os, "pwrite")
 
-_POSITIONAL_LOCK = threading.RLock()
+#: One lock per descriptor, created on demand (issue #8: the original
+#: module-level lock serialised every open disk behind one RLock).  Entries
+#: are intentionally not pruned: an fd number reused after close shares its
+#: predecessor's lock, which can only add contention, never break correctness,
+#: and pruning would need a close hook this layer does not own.
+_PIO_LOCKS: dict = {}
+_PIO_GUARD = threading.Lock()
+
+
+def _pio_lock(fd: int) -> threading.RLock:
+    """The emulation lock for this descriptor -- same fd, same lock, always."""
+    with _PIO_GUARD:
+        lock = _PIO_LOCKS.get(fd)
+        if lock is None:
+            lock = threading.RLock()
+            _PIO_LOCKS[fd] = lock
+        return lock
 
 #: Errnos that mean "the lock is held by someone else" (or "retry the syscall"):
 #: ``flock(LOCK_NB)`` raises EAGAIN/EWOULDBLOCK (and EACCES on some systems),
@@ -134,8 +154,10 @@ def pread(fd: int, length: int, offset: int) -> bytes:
     Returns fewer bytes only at end of file, like ``os.pread``.
     """
     if _HAS_POSITIONAL and implementation() == "posix":
-        return os.pread(fd, length, offset)
-    with _POSITIONAL_LOCK:
+        # The guard above is invisible to pylint on Windows, where os has no
+        # pread/pwrite to infer from -- hence the scoped disable.
+        return os.pread(fd, length, offset)  # pylint: disable=no-member
+    with _pio_lock(fd):
         os.lseek(fd, offset, os.SEEK_SET)
         return os.read(fd, length)
 
@@ -146,8 +168,8 @@ def pwrite(fd: int, data: bytes, offset: int) -> int:
     Returns the number of bytes written, like ``os.pwrite``.
     """
     if _HAS_POSITIONAL and implementation() == "posix":
-        return os.pwrite(fd, data, offset)
-    with _POSITIONAL_LOCK:
+        return os.pwrite(fd, data, offset)  # pylint: disable=no-member
+    with _pio_lock(fd):
         os.lseek(fd, offset, os.SEEK_SET)
         return os.write(fd, data)
 

pyvdisk/sandbox.py

@@ -77,15 +77,40 @@ class _SandboxFS:
     rules, and ``fs.listdir(sandbox, "/")`` would hand the model the very paths
     ``list_files`` refuses to show it.  Paths are translated here and the calls
     are forwarded, so the capability-scoped namespace underneath still applies.
+
+    The door carries exactly the verbs defined below (plus the path-less
+    diagnostics the scoped view itself provides, such as ``df`` and ``fsck``).
+    The VScript stdlib advertises the full disk's fs API -- ``fs.symlink``,
+    ``fs.chmod`` and friends -- so an agent script that reaches for one of the
+    verbs the sandbox does not carry must be told so *in words*, not with a
+    bare AttributeError from the missing scoped method (#9).
     """
 
+    #: The verbs this door actually carries, for the refusal message.
+    VERBS = (
+        "append_file", "df", "du", "exists", "fsck", "isdir", "isfile",
+        "listdir", "listdir_with_stat", "mkdir", "makedirs", "open",
+        "read_file", "remove", "rename", "rmtree", "stat", "statfs",
+        "walk", "write_file",
+    )
+
     def __init__(self, target: Any, translate: Any):
         self._target = target
         self._translate = translate
 
     def __getattr__(self, name: str):
-        """Everything without a path (``df``, ``fsck``, ...) is passed straight on."""
-        return getattr(object.__getattribute__(self, "_target"), name)
+        """Path-less diagnostics (``df``, ``fsck``, ``statfs``) pass straight on.
+
+        Anything else the scoped view does not implement is refused with the
+        carried-verb list, so a script (and the model reading the result)
+        learns what the sandbox *does* offer instead of guessing.
+        """
+        try:
+            return getattr(object.__getattribute__(self, "_target"), name)
+        except AttributeError:
+            raise SandboxError(
+                f"fs.{name} 不在沙箱内可用；可用动词: {', '.join(self.VERBS)}"
+            ) from None
 
     @staticmethod
     def _keep(top: str, name: str) -> bool:
@@ -134,8 +159,9 @@ def isdir(self, path: str) -> bool:
         target = self._translate(path)
         return False if _hidden(target) else self._target.isdir(target)
 
-    def stat(self, path: str) -> Any:
-        return self._forward("stat", path)
+    def stat(self, path: str, *args: Any, **kwargs: Any) -> Any:
+        # follow= passes through: stdlib fs_chmod/fs_chown stat with follow.
+        return self._forward("stat", path, *args, **kwargs)
 
     def read_file(self, path: str) -> bytes:
         return self._forward("read_file", path)
@@ -161,39 +187,18 @@ def rmtree(self, path: str, *args: Any, **kwargs: Any) -> Any:
     def rename(self, source: str, destination: str, *args: Any, **kwargs: Any) -> Any:
         return self._target.rename(self._translate(source), self._translate(destination), *args, **kwargs)
 
-    def move(self, source: str, destination: str, *args: Any, **kwargs: Any) -> Any:
-        return self._target.move(self._translate(source), self._translate(destination), *args, **kwargs)
-
-    def copy(self, source: str, destination: str, *args: Any, **kwargs: Any) -> Any:
-        return self._target.copy(self._translate(source), self._translate(destination), *args, **kwargs)
-
-    def link(self, source: str, destination: str, *args: Any, **kwargs: Any) -> Any:
-        return self._target.link(self._translate(source), self._translate(destination), *args, **kwargs)
-
-    def symlink(self, source: str, destination: str, *args: Any, **kwargs: Any) -> Any:
-        return self._target.symlink(self._translate(source), self._translate(destination), *args, **kwargs)
+    # move/copy/link/symlink/readlink/truncate/chmod/chown/utime are deliberately
+    # NOT carried: the scoped view implements none of them, and the Python tool
+    # door does not offer them either.  A script that calls one now gets the
+    # refusal from __getattr__ above, with the carried-verb list, instead of a
+    # bare AttributeError that reads like a bug in the sandbox (#9).
 
     def open(self, path: str, *args: Any, **kwargs: Any) -> Any:
         return self._forward("open", path, *args, **kwargs)
 
     def du(self, path: str = "/") -> Any:
         return self._forward("du", path)
 
-    def readlink(self, path: str) -> Any:
-        return self._forward("readlink", path)
-
-    def truncate(self, path: str, *args: Any, **kwargs: Any) -> Any:
-        return self._forward("truncate", path, *args, **kwargs)
-
-    def chmod(self, path: str, *args: Any, **kwargs: Any) -> Any:
-        return self._forward("chmod", path, *args, **kwargs)
-
-    def chown(self, path: str, *args: Any, **kwargs: Any) -> Any:
-        return self._forward("chown", path, *args, **kwargs)
-
-    def utime(self, path: str, *args: Any, **kwargs: Any) -> Any:
-        return self._forward("utime", path, *args, **kwargs)
-
 
 class SandboxError(Exception):
     """The sandbox refused an operation before it reached the disk."""

tests/test_agent_sandbox.py

@@ -196,6 +196,24 @@ def test_scripts_cannot_touch_container_bookkeeping(box):
         assert not result.ok and "bookkeeping" in result.content, source
 
 
+def test_fs_verbs_outside_the_sandbox_door_refuse_with_guidance(box):
+    """The stdlib advertises the full disk's fs API; the sandbox carries a subset.
+
+    A script reaching for a verb the door does not carry (fs.symlink, fs.chmod,
+    ...) used to die with a bare AttributeError from the missing scoped method.
+    Now it is refused in words, and the words list what IS available (#9).
+    """
+    # symlink refuses immediately; chmod/truncate first stat/read the target,
+    # so each source creates its file before reaching the refusal.
+    for source in ('import std.fs as fs; fs.symlink(sandbox, "/a", "/b");',
+                   'import std.fs as fs; fs.write(sandbox, "/a.txt", "x"); fs.chmod(sandbox, "/a.txt", 420);',
+                   'import std.fs as fs; fs.write(sandbox, "/b.txt", "x"); fs.truncate(sandbox, "/b.txt", 0);'):
+        result = box.call("run_script", {"source": source})
+        assert not result.ok, source
+        assert "不在沙箱内可用" in result.content, source
+        assert "write_file" in result.content and "read_file" in result.content, source
+
+
 def test_the_script_and_python_doors_share_one_workspace(box):
     """A file written by a script is an ordinary file for every other verb."""
     box.dispatch("run_script", {"source": 'import std.fs as fs; fs.write(sandbox, "/shared.txt", "from script");'})
@@ -217,8 +235,11 @@ def test_host_roots_are_opt_in(tmp_path):
     allowed.mkdir()
     (allowed / "input.txt").write_text("from the host", encoding="utf-8")
     with AgentSandbox.create(str(tmp_path / "host.vdisk"), host_read_roots=[str(allowed)]) as box:
+        # as_posix(): backslashes cannot appear in a plain VScript string
+        # (the lexer refuses \U etc.), the same rule PR #5 taught the host
+        # tests; the policy layer accepts the mixed separators on Windows.
         result = box.call("run_script", {
-            "source": f'import std.host as host; print(host.read("{allowed}/input.txt"));',
+            "source": f'import std.host as host; print(host.read("{allowed.as_posix()}/input.txt"));',
         })
         assert result.ok and "from the host" in result.content
 

tests/test_compat.py

@@ -19,6 +19,7 @@
 import os
 import re
 import sys
+import threading
 import time
 from pathlib import Path
 
@@ -98,8 +99,9 @@ def test_uid_and_gid_are_integers():
     assert isinstance(compat.uid(), int)
     assert isinstance(compat.gid(), int)
     if os.name == "posix":
-        assert compat.uid() == os.getuid()
-        assert compat.gid() == os.getgid()
+        # pylint on Windows cannot see this guard -- os has no getuid there.
+        assert compat.uid() == os.getuid()  # pylint: disable=no-member
+        assert compat.gid() == os.getgid()  # pylint: disable=no-member
     else:
         assert (compat.uid(), compat.gid()) == (0, 0)
 
@@ -232,6 +234,48 @@ def test_emulated_positional_io_matches_the_native_one(tmp_path, windows_emulati
         os.close(fd)
 
 
+def test_positional_io_lock_is_per_descriptor(windows_emulation):
+    """Same fd shares one lock; different fds get different locks (#8).
+
+    The pre-#8 module-level lock returned the same RLock for every handle,
+    which serialised separate disks against each other on Windows.  The
+    assertion only needs descriptors, not files -- a pipe pair provides them.
+    """
+    r, w = os.pipe()
+    try:
+        assert compat._pio_lock(w) is compat._pio_lock(w), "same fd, same lock"
+        assert compat._pio_lock(w) is not compat._pio_lock(r), "different fds must not share a lock"
+    finally:
+        os.close(r)
+        os.close(w)
+
+
+def test_pio_on_one_descriptor_does_not_block_another(tmp_path, windows_emulation):
+    """While handle A is mid-pwrite, handle B must still make progress (#8).
+
+    Regression for the module-level lock: under it, the worker thread would
+    wait for A's lock and never finish.  This holds a lock on one descriptor
+    and proves IO on another descriptor completes regardless.  The descriptors
+    must be seekable files: the emulated pwrite is lseek + write, and pipes
+    would fail with ESPIPE regardless of locking.
+    """
+    a = os.open(str(tmp_path / "a.bin"), os.O_CREAT | os.O_RDWR)
+    b = os.open(str(tmp_path / "b.bin"), os.O_CREAT | os.O_RDWR)
+    done = []
+    worker = threading.Thread(
+        target=lambda: (compat.pwrite(b, b"ok", 0), done.append(True))
+    )
+    with compat._pio_lock(a):  # simulate disk A with a block IO in flight
+        worker.start()
+        worker.join(timeout=5)
+    try:
+        assert done == [1], "IO on descriptor b must not wait for descriptor a's lock"
+        assert compat.pread(b, 2, 0) == b"ok"
+    finally:
+        os.close(a)
+        os.close(b)
+
+
 def test_emulated_locking_contends_and_times_out(tmp_path, windows_emulation):
     first, second = _lock_file(tmp_path), _lock_file(tmp_path)
     try: