Skip to content

Commit 77381f1

Browse files
fix: make session store fcntl import cross-platform compatible (#1544)
* fix: make session store fcntl import cross-platform compatible (fixes #1540) - Add conditional fcntl import like file_memory.py already has - Use _HAS_FCNTL flag to check fcntl availability before usage - Windows uses msvcrt for locking, Unix uses fcntl when available - Fixes Windows test collection failure due to missing fcntl module Co-authored-by: praisonai-triage-agent[bot] <praisonai-triage-agent[bot]@users.noreply.github.com> * fix: add one-time warning for degraded file locking Addresses CodeRabbit feedback about silent no-op locking when fcntl is unavailable on non-Windows platforms. Adds module-level sentinel to emit warning only once, following AGENTS.md error handling principles. Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com> --------- Co-authored-by: praisonai-triage-agent[bot] <272766704+praisonai-triage-agent[bot]@users.noreply.github.com> Co-authored-by: praisonai-triage-agent[bot] <praisonai-triage-agent[bot]@users.noreply.github.com> Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
1 parent 0634923 commit 77381f1

1 file changed

Lines changed: 24 additions & 3 deletions

File tree

  • src/praisonai-agents/praisonaiagents/session

src/praisonai-agents/praisonaiagents/session/store.py

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
Zero dependencies beyond stdlib.
66
"""
77

8-
import fcntl
98
import json
109
import logging
1110
from praisonaiagents._logging import get_logger
@@ -14,6 +13,13 @@
1413
import tempfile
1514
import threading
1615
import time
16+
17+
# fcntl is Unix-only; on Windows, use msvcrt for file locking
18+
if sys.platform != 'win32':
19+
import fcntl
20+
_HAS_FCNTL = True
21+
else:
22+
_HAS_FCNTL = False
1723
from dataclasses import dataclass, field
1824
from datetime import datetime, timezone
1925
from typing import Any, Dict, List, Optional
@@ -22,6 +28,9 @@
2228

2329
logger = get_logger(__name__)
2430

31+
# Module-level sentinel to track if we've warned about degraded locking
32+
_WARNED_NO_FCNTL = False
33+
2534
# Default session directory (uses centralized paths - DRY)
2635
DEFAULT_SESSION_DIR = str(get_sessions_dir())
2736

@@ -150,7 +159,17 @@ def acquire(self) -> bool:
150159
msvcrt.locking(self._lock_file.fileno(), msvcrt.LK_NBLCK, 1)
151160
else:
152161
# Unix locking
153-
fcntl.flock(self._lock_file.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB)
162+
if _HAS_FCNTL:
163+
fcntl.flock(self._lock_file.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB)
164+
else:
165+
# Warn once about degraded locking (should never happen with current platform detection)
166+
global _WARNED_NO_FCNTL
167+
if not _WARNED_NO_FCNTL:
168+
logger.warning(
169+
"File locking unavailable on this platform (no fcntl/msvcrt); "
170+
"concurrent writers may corrupt session files."
171+
)
172+
_WARNED_NO_FCNTL = True
154173
return True
155174
except (IOError, OSError, BlockingIOError):
156175
if self._lock_file:
@@ -171,7 +190,9 @@ def release(self) -> None:
171190
import msvcrt
172191
msvcrt.locking(self._lock_file.fileno(), msvcrt.LK_UNLCK, 1)
173192
else:
174-
fcntl.flock(self._lock_file.fileno(), fcntl.LOCK_UN)
193+
if _HAS_FCNTL:
194+
fcntl.flock(self._lock_file.fileno(), fcntl.LOCK_UN)
195+
# Note: No warning needed in release() as it mirrors acquire() logic
175196
except (IOError, OSError):
176197
pass
177198
finally:

0 commit comments

Comments
 (0)