mirror of
https://github.com/HKUDS/CLI-Anything.git
synced 2026-10-29 17:43:44 +08:00
Merge pull request #94 from ZJZAC/fix/session-race-condition
fix(session): atomic file locking to prevent race conditions in all harnesses
This commit is contained in:
@@ -1,11 +1,38 @@
|
||||
"""Session management — undo/redo and command history for AnyGen CLI."""
|
||||
|
||||
import json
|
||||
import os
|
||||
from dataclasses import dataclass, field
|
||||
from datetime import datetime, timezone
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
def _locked_save_json(path, data, **dump_kwargs) -> None:
|
||||
"""Atomically write JSON with exclusive file locking."""
|
||||
try:
|
||||
f = open(path, "r+")
|
||||
except FileNotFoundError:
|
||||
os.makedirs(os.path.dirname(os.path.abspath(path)), exist_ok=True)
|
||||
f = open(path, "w")
|
||||
with f:
|
||||
_locked = False
|
||||
try:
|
||||
import fcntl
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_EX)
|
||||
_locked = True
|
||||
except (ImportError, OSError):
|
||||
pass
|
||||
try:
|
||||
f.seek(0)
|
||||
f.truncate()
|
||||
json.dump(data, f, **dump_kwargs)
|
||||
f.flush()
|
||||
finally:
|
||||
if _locked:
|
||||
import fcntl
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_UN)
|
||||
|
||||
|
||||
@dataclass
|
||||
class HistoryEntry:
|
||||
command: str
|
||||
@@ -101,9 +128,7 @@ class Session:
|
||||
"history": [e.to_dict() for e in self._history],
|
||||
"redo_stack": [e.to_dict() for e in self._redo_stack],
|
||||
}
|
||||
Path(path).parent.mkdir(parents=True, exist_ok=True)
|
||||
with open(path, "w") as f:
|
||||
json.dump(data, f, indent=2, sort_keys=True, default=str)
|
||||
_locked_save_json(path, data, indent=2, sort_keys=True, default=str)
|
||||
|
||||
def _load(self, path: str):
|
||||
p = Path(path)
|
||||
|
||||
@@ -8,6 +8,32 @@ import json
|
||||
import os
|
||||
import copy
|
||||
from typing import Dict, Any, Optional, List
|
||||
|
||||
|
||||
def _locked_save_json(path, data, **dump_kwargs) -> None:
|
||||
"""Atomically write JSON with exclusive file locking."""
|
||||
try:
|
||||
f = open(path, "r+")
|
||||
except FileNotFoundError:
|
||||
os.makedirs(os.path.dirname(os.path.abspath(path)), exist_ok=True)
|
||||
f = open(path, "w")
|
||||
with f:
|
||||
_locked = False
|
||||
try:
|
||||
import fcntl
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_EX)
|
||||
_locked = True
|
||||
except (ImportError, OSError):
|
||||
pass
|
||||
try:
|
||||
f.seek(0)
|
||||
f.truncate()
|
||||
json.dump(data, f, **dump_kwargs)
|
||||
f.flush()
|
||||
finally:
|
||||
if _locked:
|
||||
import fcntl
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_UN)
|
||||
from datetime import datetime
|
||||
|
||||
|
||||
@@ -119,9 +145,7 @@ class Session:
|
||||
|
||||
# Save project
|
||||
self.project["metadata"]["modified"] = datetime.now().isoformat()
|
||||
os.makedirs(os.path.dirname(os.path.abspath(save_path)), exist_ok=True)
|
||||
with open(save_path, "w") as f:
|
||||
json.dump(self.project, f, indent=2, sort_keys=True, default=str)
|
||||
_locked_save_json(save_path, self.project, indent=2, sort_keys=True, default=str)
|
||||
|
||||
self.project_path = save_path
|
||||
self._modified = False
|
||||
|
||||
@@ -7,6 +7,32 @@ from typing import Dict, Any, Optional, List
|
||||
from datetime import datetime
|
||||
|
||||
|
||||
def _locked_save_json(path, data, **dump_kwargs) -> None:
|
||||
"""Atomically write JSON with exclusive file locking."""
|
||||
try:
|
||||
f = open(path, "r+")
|
||||
except FileNotFoundError:
|
||||
os.makedirs(os.path.dirname(os.path.abspath(path)), exist_ok=True)
|
||||
f = open(path, "w")
|
||||
with f:
|
||||
_locked = False
|
||||
try:
|
||||
import fcntl
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_EX)
|
||||
_locked = True
|
||||
except (ImportError, OSError):
|
||||
pass
|
||||
try:
|
||||
f.seek(0)
|
||||
f.truncate()
|
||||
json.dump(data, f, **dump_kwargs)
|
||||
f.flush()
|
||||
finally:
|
||||
if _locked:
|
||||
import fcntl
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_UN)
|
||||
|
||||
|
||||
class Session:
|
||||
"""Manages project state with undo/redo history."""
|
||||
|
||||
@@ -111,8 +137,7 @@ class Session:
|
||||
|
||||
# Save project
|
||||
self.project["metadata"]["modified"] = datetime.now().isoformat()
|
||||
with open(save_path, "w") as f:
|
||||
json.dump(self.project, f, indent=2, sort_keys=True, default=str)
|
||||
_locked_save_json(save_path, self.project, indent=2, sort_keys=True, default=str)
|
||||
|
||||
self.project_path = save_path
|
||||
self._modified = False
|
||||
|
||||
@@ -67,6 +67,36 @@ designed for humans, without needing a display or mouse.
|
||||
5. **Add rendering/export** — The export pipeline calls the backend module.
|
||||
Generate valid intermediate files, then invoke the real software for conversion.
|
||||
6. **Add session management** — State persistence, undo/redo
|
||||
|
||||
**Session file locking** — When saving session JSON, use exclusive file locking
|
||||
to prevent concurrent writes from corrupting data. Never use bare
|
||||
`open("w") + json.dump()` — `open("w")` truncates the file before any lock
|
||||
can be acquired. Instead, open with `"r+"`, lock, then truncate inside the lock:
|
||||
```python
|
||||
def _locked_save_json(path, data, **dump_kwargs) -> None:
|
||||
"""Atomically write JSON with exclusive file locking."""
|
||||
try:
|
||||
f = open(path, "r+") # no truncation on open
|
||||
except FileNotFoundError:
|
||||
os.makedirs(os.path.dirname(os.path.abspath(path)), exist_ok=True)
|
||||
f = open(path, "w") # first save — file doesn't exist yet
|
||||
with f:
|
||||
_locked = False
|
||||
try:
|
||||
import fcntl
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_EX)
|
||||
_locked = True
|
||||
except (ImportError, OSError):
|
||||
pass # Windows / unsupported FS — proceed unlocked
|
||||
try:
|
||||
f.seek(0)
|
||||
f.truncate() # truncate INSIDE the lock
|
||||
json.dump(data, f, **dump_kwargs)
|
||||
f.flush()
|
||||
finally:
|
||||
if _locked:
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_UN)
|
||||
```
|
||||
7. **Add the REPL with unified skin** — Interactive mode wrapping the subcommands.
|
||||
- Copy `repl_skin.py` from the plugin (`cli-anything-plugin/repl_skin.py`) into
|
||||
`utils/repl_skin.py` in your CLI package
|
||||
|
||||
@@ -14,6 +14,33 @@ from xml.etree import ElementTree as ET
|
||||
from ..utils import drawio_xml
|
||||
|
||||
|
||||
def _locked_save_json(path, data, **dump_kwargs) -> None:
|
||||
"""Atomically write JSON with exclusive file locking."""
|
||||
path = str(path)
|
||||
try:
|
||||
f = open(path, "r+")
|
||||
except FileNotFoundError:
|
||||
os.makedirs(os.path.dirname(os.path.abspath(path)), exist_ok=True)
|
||||
f = open(path, "w")
|
||||
with f:
|
||||
_locked = False
|
||||
try:
|
||||
import fcntl
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_EX)
|
||||
_locked = True
|
||||
except (ImportError, OSError):
|
||||
pass
|
||||
try:
|
||||
f.seek(0)
|
||||
f.truncate()
|
||||
json.dump(data, f, **dump_kwargs)
|
||||
f.flush()
|
||||
finally:
|
||||
if _locked:
|
||||
import fcntl
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_UN)
|
||||
|
||||
|
||||
SESSION_DIR = Path.home() / ".drawio-cli" / "sessions"
|
||||
MAX_UNDO_DEPTH = 50
|
||||
|
||||
@@ -126,8 +153,7 @@ class Session:
|
||||
"timestamp": time.time(),
|
||||
}
|
||||
path = SESSION_DIR / f"{self.session_id}.json"
|
||||
with open(path, "w") as f:
|
||||
json.dump(state, f, indent=2, sort_keys=True)
|
||||
_locked_save_json(path, state, indent=2, sort_keys=True)
|
||||
return str(path)
|
||||
|
||||
@classmethod
|
||||
|
||||
@@ -7,6 +7,32 @@ from typing import Dict, Any, Optional, List
|
||||
from datetime import datetime
|
||||
|
||||
|
||||
def _locked_save_json(path, data, **dump_kwargs) -> None:
|
||||
"""Atomically write JSON with exclusive file locking."""
|
||||
try:
|
||||
f = open(path, "r+")
|
||||
except FileNotFoundError:
|
||||
os.makedirs(os.path.dirname(os.path.abspath(path)), exist_ok=True)
|
||||
f = open(path, "w")
|
||||
with f:
|
||||
_locked = False
|
||||
try:
|
||||
import fcntl
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_EX)
|
||||
_locked = True
|
||||
except (ImportError, OSError):
|
||||
pass
|
||||
try:
|
||||
f.seek(0)
|
||||
f.truncate()
|
||||
json.dump(data, f, **dump_kwargs)
|
||||
f.flush()
|
||||
finally:
|
||||
if _locked:
|
||||
import fcntl
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_UN)
|
||||
|
||||
|
||||
class Session:
|
||||
"""Manages project state with undo/redo history."""
|
||||
|
||||
@@ -111,8 +137,7 @@ class Session:
|
||||
|
||||
# Save project
|
||||
self.project["metadata"]["modified"] = datetime.now().isoformat()
|
||||
with open(save_path, "w") as f:
|
||||
json.dump(self.project, f, indent=2, sort_keys=True, default=str)
|
||||
_locked_save_json(save_path, self.project, indent=2, sort_keys=True, default=str)
|
||||
|
||||
self.project_path = save_path
|
||||
self._modified = False
|
||||
|
||||
@@ -476,3 +476,76 @@ class TestSession:
|
||||
for i in range(10):
|
||||
sess.snapshot(f"action {i}")
|
||||
assert len(sess._undo_stack) == 5
|
||||
|
||||
|
||||
# ── Concurrent Save Tests ───────────────────────────────────────
|
||||
|
||||
class TestLockedSaveJson:
|
||||
"""Tests for _locked_save_json atomic file writes."""
|
||||
|
||||
def test_basic_save(self, tmp_path):
|
||||
from cli_anything.gimp.core.session import _locked_save_json
|
||||
path = str(tmp_path / "test.json")
|
||||
_locked_save_json(path, {"key": "value"}, indent=2)
|
||||
with open(path) as f:
|
||||
data = json.load(f)
|
||||
assert data == {"key": "value"}
|
||||
|
||||
def test_overwrite_existing(self, tmp_path):
|
||||
from cli_anything.gimp.core.session import _locked_save_json
|
||||
path = str(tmp_path / "test.json")
|
||||
_locked_save_json(path, {"version": 1}, indent=2)
|
||||
_locked_save_json(path, {"version": 2}, indent=2)
|
||||
with open(path) as f:
|
||||
data = json.load(f)
|
||||
assert data == {"version": 2}
|
||||
|
||||
def test_overwrite_shorter_data(self, tmp_path):
|
||||
"""Ensure truncation works — shorter data doesn't leave old bytes."""
|
||||
from cli_anything.gimp.core.session import _locked_save_json
|
||||
path = str(tmp_path / "test.json")
|
||||
_locked_save_json(path, {"key": "a" * 1000}, indent=2)
|
||||
_locked_save_json(path, {"k": 1}, indent=2)
|
||||
with open(path) as f:
|
||||
data = json.load(f)
|
||||
assert data == {"k": 1}
|
||||
|
||||
def test_creates_parent_dirs(self, tmp_path):
|
||||
from cli_anything.gimp.core.session import _locked_save_json
|
||||
path = str(tmp_path / "nested" / "dir" / "test.json")
|
||||
_locked_save_json(path, {"nested": True})
|
||||
with open(path) as f:
|
||||
data = json.load(f)
|
||||
assert data == {"nested": True}
|
||||
|
||||
def test_concurrent_writes_produce_valid_json(self, tmp_path):
|
||||
"""Multiple threads writing to the same file should not corrupt it."""
|
||||
from cli_anything.gimp.core.session import _locked_save_json
|
||||
import threading
|
||||
|
||||
path = str(tmp_path / "concurrent.json")
|
||||
errors = []
|
||||
|
||||
def writer(thread_id):
|
||||
try:
|
||||
for i in range(50):
|
||||
_locked_save_json(
|
||||
path,
|
||||
{"thread": thread_id, "iteration": i},
|
||||
indent=2, sort_keys=True,
|
||||
)
|
||||
except Exception as e:
|
||||
errors.append(e)
|
||||
|
||||
threads = [threading.Thread(target=writer, args=(t,)) for t in range(4)]
|
||||
for t in threads:
|
||||
t.start()
|
||||
for t in threads:
|
||||
t.join()
|
||||
|
||||
assert not errors, f"Errors during concurrent writes: {errors}"
|
||||
# Final file must be valid JSON
|
||||
with open(path) as f:
|
||||
data = json.load(f)
|
||||
assert "thread" in data
|
||||
assert "iteration" in data
|
||||
|
||||
@@ -7,6 +7,32 @@ from typing import Dict, Any, Optional, List
|
||||
from datetime import datetime
|
||||
|
||||
|
||||
def _locked_save_json(path, data, **dump_kwargs) -> None:
|
||||
"""Atomically write JSON with exclusive file locking."""
|
||||
try:
|
||||
f = open(path, "r+")
|
||||
except FileNotFoundError:
|
||||
os.makedirs(os.path.dirname(os.path.abspath(path)), exist_ok=True)
|
||||
f = open(path, "w")
|
||||
with f:
|
||||
_locked = False
|
||||
try:
|
||||
import fcntl
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_EX)
|
||||
_locked = True
|
||||
except (ImportError, OSError):
|
||||
pass
|
||||
try:
|
||||
f.seek(0)
|
||||
f.truncate()
|
||||
json.dump(data, f, **dump_kwargs)
|
||||
f.flush()
|
||||
finally:
|
||||
if _locked:
|
||||
import fcntl
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_UN)
|
||||
|
||||
|
||||
class Session:
|
||||
"""Manages project state with undo/redo history."""
|
||||
|
||||
@@ -110,9 +136,7 @@ class Session:
|
||||
raise ValueError("No save path specified.")
|
||||
|
||||
self.project["metadata"]["modified"] = datetime.now().isoformat()
|
||||
os.makedirs(os.path.dirname(os.path.abspath(save_path)), exist_ok=True)
|
||||
with open(save_path, "w") as f:
|
||||
json.dump(self.project, f, indent=2, sort_keys=True, default=str)
|
||||
_locked_save_json(save_path, self.project, indent=2, sort_keys=True, default=str)
|
||||
|
||||
self.project_path = save_path
|
||||
self._modified = False
|
||||
|
||||
@@ -7,6 +7,32 @@ from typing import Dict, Any, Optional, List
|
||||
from datetime import datetime
|
||||
|
||||
|
||||
def _locked_save_json(path, data, **dump_kwargs) -> None:
|
||||
"""Atomically write JSON with exclusive file locking."""
|
||||
try:
|
||||
f = open(path, "r+")
|
||||
except FileNotFoundError:
|
||||
os.makedirs(os.path.dirname(os.path.abspath(path)), exist_ok=True)
|
||||
f = open(path, "w")
|
||||
with f:
|
||||
_locked = False
|
||||
try:
|
||||
import fcntl
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_EX)
|
||||
_locked = True
|
||||
except (ImportError, OSError):
|
||||
pass
|
||||
try:
|
||||
f.seek(0)
|
||||
f.truncate()
|
||||
json.dump(data, f, **dump_kwargs)
|
||||
f.flush()
|
||||
finally:
|
||||
if _locked:
|
||||
import fcntl
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_UN)
|
||||
|
||||
|
||||
class Session:
|
||||
"""Manages project state with undo/redo history."""
|
||||
|
||||
@@ -106,11 +132,7 @@ class Session:
|
||||
raise ValueError("No save path specified.")
|
||||
|
||||
self.project["metadata"]["modified"] = datetime.now().isoformat()
|
||||
parent = os.path.dirname(os.path.abspath(save_path))
|
||||
if parent:
|
||||
os.makedirs(parent, exist_ok=True)
|
||||
with open(save_path, "w") as f:
|
||||
json.dump(self.project, f, indent=2, sort_keys=True, default=str)
|
||||
_locked_save_json(save_path, self.project, indent=2, sort_keys=True, default=str)
|
||||
|
||||
self.project_path = save_path
|
||||
self._modified = False
|
||||
|
||||
@@ -7,6 +7,32 @@ from typing import Dict, Any, Optional, List
|
||||
from datetime import datetime
|
||||
|
||||
|
||||
def _locked_save_json(path, data, **dump_kwargs) -> None:
|
||||
"""Atomically write JSON with exclusive file locking."""
|
||||
try:
|
||||
f = open(path, "r+")
|
||||
except FileNotFoundError:
|
||||
os.makedirs(os.path.dirname(os.path.abspath(path)), exist_ok=True)
|
||||
f = open(path, "w")
|
||||
with f:
|
||||
_locked = False
|
||||
try:
|
||||
import fcntl
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_EX)
|
||||
_locked = True
|
||||
except (ImportError, OSError):
|
||||
pass
|
||||
try:
|
||||
f.seek(0)
|
||||
f.truncate()
|
||||
json.dump(data, f, **dump_kwargs)
|
||||
f.flush()
|
||||
finally:
|
||||
if _locked:
|
||||
import fcntl
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_UN)
|
||||
|
||||
|
||||
class Session:
|
||||
"""Manages document project state with undo/redo history."""
|
||||
|
||||
@@ -117,9 +143,7 @@ class Session:
|
||||
raise ValueError("No save path specified.")
|
||||
|
||||
self.project["metadata"]["modified"] = datetime.now().isoformat()
|
||||
os.makedirs(os.path.dirname(os.path.abspath(save_path)), exist_ok=True)
|
||||
with open(save_path, "w") as f:
|
||||
json.dump(self.project, f, indent=2, sort_keys=True, default=str)
|
||||
_locked_save_json(save_path, self.project, indent=2, sort_keys=True, default=str)
|
||||
|
||||
self.project_path = save_path
|
||||
self._modified = False
|
||||
|
||||
@@ -7,6 +7,32 @@ from typing import Dict, Any, Optional, List
|
||||
from datetime import datetime
|
||||
|
||||
|
||||
def _locked_save_json(path, data, **dump_kwargs) -> None:
|
||||
"""Atomically write JSON with exclusive file locking."""
|
||||
try:
|
||||
f = open(path, "r+")
|
||||
except FileNotFoundError:
|
||||
os.makedirs(os.path.dirname(os.path.abspath(path)), exist_ok=True)
|
||||
f = open(path, "w")
|
||||
with f:
|
||||
_locked = False
|
||||
try:
|
||||
import fcntl
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_EX)
|
||||
_locked = True
|
||||
except (ImportError, OSError):
|
||||
pass
|
||||
try:
|
||||
f.seek(0)
|
||||
f.truncate()
|
||||
json.dump(data, f, **dump_kwargs)
|
||||
f.flush()
|
||||
finally:
|
||||
if _locked:
|
||||
import fcntl
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_UN)
|
||||
|
||||
|
||||
class Session:
|
||||
"""Manages project state with undo/redo history."""
|
||||
|
||||
@@ -106,9 +132,7 @@ class Session:
|
||||
raise ValueError("No save path specified.")
|
||||
|
||||
self.project["metadata"]["modified"] = datetime.now().isoformat()
|
||||
os.makedirs(os.path.dirname(os.path.abspath(save_path)), exist_ok=True)
|
||||
with open(save_path, "w") as f:
|
||||
json.dump(self.project, f, indent=2, sort_keys=True, default=str)
|
||||
_locked_save_json(save_path, self.project, indent=2, sort_keys=True, default=str)
|
||||
|
||||
self.project_path = save_path
|
||||
self._modified = False
|
||||
|
||||
@@ -15,6 +15,33 @@ from lxml import etree
|
||||
from ..utils import mlt_xml
|
||||
|
||||
|
||||
def _locked_save_json(path, data, **dump_kwargs) -> None:
|
||||
"""Atomically write JSON with exclusive file locking."""
|
||||
path = str(path)
|
||||
try:
|
||||
f = open(path, "r+")
|
||||
except FileNotFoundError:
|
||||
os.makedirs(os.path.dirname(os.path.abspath(path)), exist_ok=True)
|
||||
f = open(path, "w")
|
||||
with f:
|
||||
_locked = False
|
||||
try:
|
||||
import fcntl
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_EX)
|
||||
_locked = True
|
||||
except (ImportError, OSError):
|
||||
pass
|
||||
try:
|
||||
f.seek(0)
|
||||
f.truncate()
|
||||
json.dump(data, f, **dump_kwargs)
|
||||
f.flush()
|
||||
finally:
|
||||
if _locked:
|
||||
import fcntl
|
||||
fcntl.flock(f.fileno(), fcntl.LOCK_UN)
|
||||
|
||||
|
||||
SESSION_DIR = Path.home() / ".shotcut-cli" / "sessions"
|
||||
MAX_UNDO_DEPTH = 50
|
||||
|
||||
@@ -154,8 +181,7 @@ class Session:
|
||||
"timestamp": time.time(),
|
||||
}
|
||||
path = SESSION_DIR / f"{self.session_id}.json"
|
||||
with open(path, "w") as f:
|
||||
json.dump(state, f, indent=2, sort_keys=True)
|
||||
_locked_save_json(path, state, indent=2, sort_keys=True)
|
||||
return str(path)
|
||||
|
||||
@classmethod
|
||||
|
||||
Reference in New Issue
Block a user