mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-05-23 16:35:59 +00:00
fix(harness): reset local sandbox singleton with provider lifecycle (#2834)
* Fix local sandbox singleton reset on provider lifecycle * Fix local sandbox singleton reset on provider reset --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
@@ -119,3 +119,13 @@ class LocalSandboxProvider(SandboxProvider):
|
|||||||
# For Docker-based providers (e.g., AioSandboxProvider), cleanup
|
# For Docker-based providers (e.g., AioSandboxProvider), cleanup
|
||||||
# happens at application shutdown via the shutdown() method.
|
# happens at application shutdown via the shutdown() method.
|
||||||
pass
|
pass
|
||||||
|
|
||||||
|
def reset(self) -> None:
|
||||||
|
# reset_sandbox_provider() must also clear the module singleton.
|
||||||
|
global _singleton
|
||||||
|
_singleton = None
|
||||||
|
|
||||||
|
def shutdown(self) -> None:
|
||||||
|
# LocalSandboxProvider has no extra resources beyond the shared
|
||||||
|
# singleton, so shutdown uses the same cleanup path as reset.
|
||||||
|
self.reset()
|
||||||
|
|||||||
@@ -37,6 +37,10 @@ class SandboxProvider(ABC):
|
|||||||
"""
|
"""
|
||||||
pass
|
pass
|
||||||
|
|
||||||
|
def reset(self) -> None:
|
||||||
|
"""Clear cached state that survives provider instance replacement."""
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
_default_sandbox_provider: SandboxProvider | None = None
|
_default_sandbox_provider: SandboxProvider | None = None
|
||||||
|
|
||||||
@@ -65,11 +69,18 @@ def reset_sandbox_provider() -> None:
|
|||||||
The next call to `get_sandbox_provider()` will create a new instance.
|
The next call to `get_sandbox_provider()` will create a new instance.
|
||||||
Useful for testing or when switching configurations.
|
Useful for testing or when switching configurations.
|
||||||
|
|
||||||
|
Providers can override `reset()` to clear any module-level state they keep
|
||||||
|
alive across instances (for example, `LocalSandboxProvider`'s cached
|
||||||
|
`LocalSandbox` singleton). Without it, config/mount changes would not take
|
||||||
|
effect on the next acquire().
|
||||||
|
|
||||||
Note: If the provider has active sandboxes, they will be orphaned.
|
Note: If the provider has active sandboxes, they will be orphaned.
|
||||||
Use `shutdown_sandbox_provider()` for proper cleanup.
|
Use `shutdown_sandbox_provider()` for proper cleanup.
|
||||||
"""
|
"""
|
||||||
global _default_sandbox_provider
|
global _default_sandbox_provider
|
||||||
_default_sandbox_provider = None
|
if _default_sandbox_provider is not None:
|
||||||
|
_default_sandbox_provider.reset()
|
||||||
|
_default_sandbox_provider = None
|
||||||
|
|
||||||
|
|
||||||
def shutdown_sandbox_provider() -> None:
|
def shutdown_sandbox_provider() -> None:
|
||||||
|
|||||||
@@ -639,3 +639,148 @@ class TestLocalSandboxProviderMounts:
|
|||||||
provider = LocalSandboxProvider()
|
provider = LocalSandboxProvider()
|
||||||
|
|
||||||
assert [m.container_path for m in provider._path_mappings] == ["/mnt/skills", "/mnt/data"]
|
assert [m.container_path for m in provider._path_mappings] == ["/mnt/skills", "/mnt/data"]
|
||||||
|
|
||||||
|
|
||||||
|
class TestLocalSandboxProviderResetClearsSingleton:
|
||||||
|
"""Regression coverage for issue #2815.
|
||||||
|
|
||||||
|
The module-level LocalSandbox singleton must be cleared whenever the
|
||||||
|
provider is reset or shut down — otherwise stale path mappings and
|
||||||
|
mount policy survive config reloads and test teardown.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def _build_config(self, skills_dir, mounts):
|
||||||
|
from deerflow.config.sandbox_config import SandboxConfig
|
||||||
|
|
||||||
|
sandbox_config = SandboxConfig(
|
||||||
|
use="deerflow.sandbox.local:LocalSandboxProvider",
|
||||||
|
mounts=mounts,
|
||||||
|
)
|
||||||
|
return SimpleNamespace(
|
||||||
|
skills=SimpleNamespace(
|
||||||
|
container_path="/mnt/skills",
|
||||||
|
get_skills_path=lambda: skills_dir,
|
||||||
|
use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage",
|
||||||
|
),
|
||||||
|
sandbox=sandbox_config,
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_reset_sandbox_provider_clears_local_singleton(self, tmp_path):
|
||||||
|
from deerflow.config.sandbox_config import VolumeMountConfig
|
||||||
|
from deerflow.sandbox import local as local_module
|
||||||
|
from deerflow.sandbox.local import local_sandbox_provider as lsp_module
|
||||||
|
from deerflow.sandbox.sandbox_provider import (
|
||||||
|
get_sandbox_provider,
|
||||||
|
reset_sandbox_provider,
|
||||||
|
)
|
||||||
|
|
||||||
|
skills_dir = tmp_path / "skills"
|
||||||
|
skills_dir.mkdir()
|
||||||
|
first_dir = tmp_path / "first"
|
||||||
|
first_dir.mkdir()
|
||||||
|
second_dir = tmp_path / "second"
|
||||||
|
second_dir.mkdir()
|
||||||
|
|
||||||
|
first_cfg = self._build_config(
|
||||||
|
skills_dir,
|
||||||
|
[VolumeMountConfig(host_path=str(first_dir), container_path="/mnt/first", read_only=False)],
|
||||||
|
)
|
||||||
|
second_cfg = self._build_config(
|
||||||
|
skills_dir,
|
||||||
|
[VolumeMountConfig(host_path=str(second_dir), container_path="/mnt/second", read_only=False)],
|
||||||
|
)
|
||||||
|
|
||||||
|
# Make sure no leftover singleton from a prior test interferes.
|
||||||
|
lsp_module._singleton = None
|
||||||
|
reset_sandbox_provider()
|
||||||
|
|
||||||
|
try:
|
||||||
|
with patch("deerflow.sandbox.sandbox_provider.get_app_config", return_value=first_cfg), patch("deerflow.config.get_app_config", return_value=first_cfg):
|
||||||
|
provider = get_sandbox_provider()
|
||||||
|
provider.acquire()
|
||||||
|
|
||||||
|
assert lsp_module._singleton is not None
|
||||||
|
first_container_paths = {m.container_path for m in lsp_module._singleton.path_mappings}
|
||||||
|
assert "/mnt/first" in first_container_paths
|
||||||
|
|
||||||
|
reset_sandbox_provider()
|
||||||
|
|
||||||
|
# The whole point of the regression: reset must drop the cached LocalSandbox.
|
||||||
|
assert lsp_module._singleton is None
|
||||||
|
|
||||||
|
with patch("deerflow.sandbox.sandbox_provider.get_app_config", return_value=second_cfg), patch("deerflow.config.get_app_config", return_value=second_cfg):
|
||||||
|
provider2 = get_sandbox_provider()
|
||||||
|
provider2.acquire()
|
||||||
|
|
||||||
|
assert provider2 is not provider
|
||||||
|
second_container_paths = {m.container_path for m in lsp_module._singleton.path_mappings}
|
||||||
|
assert "/mnt/second" in second_container_paths
|
||||||
|
assert "/mnt/first" not in second_container_paths
|
||||||
|
finally:
|
||||||
|
lsp_module._singleton = None
|
||||||
|
reset_sandbox_provider()
|
||||||
|
|
||||||
|
# Sanity: the local sandbox module still exposes the singleton symbol
|
||||||
|
# at the same module path (guards against accidental rename).
|
||||||
|
assert hasattr(local_module.local_sandbox_provider, "_singleton")
|
||||||
|
|
||||||
|
def test_shutdown_sandbox_provider_clears_local_singleton(self, tmp_path):
|
||||||
|
from deerflow.config.sandbox_config import VolumeMountConfig
|
||||||
|
from deerflow.sandbox.local import local_sandbox_provider as lsp_module
|
||||||
|
from deerflow.sandbox.sandbox_provider import (
|
||||||
|
get_sandbox_provider,
|
||||||
|
reset_sandbox_provider,
|
||||||
|
shutdown_sandbox_provider,
|
||||||
|
)
|
||||||
|
|
||||||
|
skills_dir = tmp_path / "skills"
|
||||||
|
skills_dir.mkdir()
|
||||||
|
mount_dir = tmp_path / "mount"
|
||||||
|
mount_dir.mkdir()
|
||||||
|
|
||||||
|
cfg = self._build_config(
|
||||||
|
skills_dir,
|
||||||
|
[VolumeMountConfig(host_path=str(mount_dir), container_path="/mnt/data", read_only=False)],
|
||||||
|
)
|
||||||
|
|
||||||
|
lsp_module._singleton = None
|
||||||
|
reset_sandbox_provider()
|
||||||
|
|
||||||
|
try:
|
||||||
|
with patch("deerflow.sandbox.sandbox_provider.get_app_config", return_value=cfg), patch("deerflow.config.get_app_config", return_value=cfg):
|
||||||
|
provider = get_sandbox_provider()
|
||||||
|
provider.acquire()
|
||||||
|
|
||||||
|
assert lsp_module._singleton is not None
|
||||||
|
|
||||||
|
shutdown_sandbox_provider()
|
||||||
|
|
||||||
|
assert lsp_module._singleton is None
|
||||||
|
finally:
|
||||||
|
lsp_module._singleton = None
|
||||||
|
reset_sandbox_provider()
|
||||||
|
|
||||||
|
def test_provider_reset_method_is_idempotent(self, tmp_path):
|
||||||
|
from deerflow.sandbox.local import local_sandbox_provider as lsp_module
|
||||||
|
from deerflow.sandbox.local.local_sandbox_provider import LocalSandboxProvider
|
||||||
|
|
||||||
|
skills_dir = tmp_path / "skills"
|
||||||
|
skills_dir.mkdir()
|
||||||
|
cfg = self._build_config(skills_dir, [])
|
||||||
|
|
||||||
|
lsp_module._singleton = None
|
||||||
|
|
||||||
|
try:
|
||||||
|
with patch("deerflow.config.get_app_config", return_value=cfg):
|
||||||
|
provider = LocalSandboxProvider()
|
||||||
|
provider.acquire()
|
||||||
|
assert lsp_module._singleton is not None
|
||||||
|
|
||||||
|
provider.reset()
|
||||||
|
assert lsp_module._singleton is None
|
||||||
|
|
||||||
|
# Calling reset again on an already-cleared singleton is safe.
|
||||||
|
provider.reset()
|
||||||
|
assert lsp_module._singleton is None
|
||||||
|
finally:
|
||||||
|
lsp_module._singleton = None
|
||||||
|
|||||||
Reference in New Issue
Block a user