RDBC-1059 Guard save_changes and request executor against use-after-dispose

Author: poissoncorp

ravendb/documents/session/document_session.py

@@ -166,6 +166,7 @@ def operations(self) -> OperationExecutor:
         return self._operation_executor
 
     def save_changes(self) -> None:
+        self.assert_not_disposed()
         save_changes_operation = BatchOperation(self)
         command = save_changes_operation.create_request()
         if command:

ravendb/documents/session/document_session_operations/in_memory_document_session_operations.py

@@ -3,6 +3,7 @@
 import datetime
 import itertools
 import json
+import os
 from abc import abstractmethod
 
 from ravendb.documents.operations.executor import OperationExecutor
@@ -62,6 +63,11 @@
     from ravendb.http.request_executor import RequestExecutor
 
 
+# Escape hatch for the cross-component store-disposed guard (the session's
+# own disposed check is unaffected). Mirrors C# DisableDisposeChecks.
+_DISABLE_DISPOSE_CHECKS: bool = os.environ.get("RAVEN_DISABLE_DISPOSE_CHECKS", "").lower() == "true"
+
+
 class RefEq:
     ref = None
 
@@ -1287,6 +1293,14 @@ def __close(self, is_disposing: bool) -> None:
     def close(self) -> None:
         self.__close(True)
 
+    def assert_not_disposed(self) -> None:
+        if self._is_disposed:
+            raise RuntimeError("The session has already been disposed and cannot be used")
+        if _DISABLE_DISPOSE_CHECKS:
+            return
+        if self._document_store.disposed:
+            raise RuntimeError("The document store has already been disposed and cannot be used")
+
     def register_missing(self, *keys: str) -> None:
         if self.no_tracking:
             return

ravendb/http/request_executor.py

@@ -497,6 +497,7 @@ def _dispose_all_failed_nodes_timers(self) -> None:
         self.__failed_nodes_timers.clear()
 
     def execute_command(self, command: RavenCommand, session_info: Optional[SessionInfo] = None) -> None:
+        self._throw_if_disposed_at_entry()
         topology_update = self._first_topology_update_task
         if (
             topology_update is not None
@@ -510,6 +511,21 @@ def execute_command(self, command: RavenCommand, session_info: Optional[SessionI
         else:
             self.__unlikely_execute(command, topology_update, session_info)
 
+    @staticmethod
+    def _throw_object_disposed() -> None:
+        raise RuntimeError("The request executor has already been disposed and cannot be used")
+
+    def _throw_if_disposed_at_entry(self) -> None:
+        # Entry guard from C# RequestExecutor.ExecuteAsync (v7.2.3).
+        from ravendb.documents.session.document_session_operations.in_memory_document_session_operations import (
+            _DISABLE_DISPOSE_CHECKS,
+        )
+
+        if _DISABLE_DISPOSE_CHECKS:
+            return
+        if self._disposed:
+            self._throw_object_disposed()
+
     def execute(
         self,
         chosen_node: ServerNode = None,

ravendb/tests/session_tests/test_disposed_guards.py

@@ -0,0 +1,76 @@
+"""
+Unit tests for the 7.2.3 disposed-guard additions:
+  * RequestExecutor.execute_command raises after the executor is disposed.
+  * RAVEN_DISABLE_DISPOSE_CHECKS env var escapes the cross-component guard.
+
+We avoid importlib.reload here on purpose — reloading
+in_memory_document_session_operations swaps out module-level classes
+(RefEq, TrackedEntitiesHolder, ...) which then trip identity checks in
+unrelated tests sharing the same process. Instead we monkey-patch the
+module-level `_DISABLE_DISPOSE_CHECKS` constant directly.
+"""
+
+import os
+import unittest
+
+import ravendb.documents.session.document_session_operations.in_memory_document_session_operations as _session_mod
+from ravendb.http.request_executor import RequestExecutor
+
+
+class TestDisableDisposeChecksEnvVar(unittest.TestCase):
+    def test_sentinel_reads_env_at_import_time(self):
+        # The constant is a plain bool computed when the module is first
+        # imported. We verify it reflects the current process env without
+        # forcing a reload (which would invalidate the class identity of
+        # RefEq / TrackedEntitiesHolder for any module that already imported
+        # those — and break unrelated tests sharing this process).
+        expected = os.environ.get("RAVEN_DISABLE_DISPOSE_CHECKS", "").lower() == "true"
+        self.assertEqual(expected, _session_mod._DISABLE_DISPOSE_CHECKS)
+
+    def test_sentinel_is_a_bool(self):
+        self.assertIsInstance(_session_mod._DISABLE_DISPOSE_CHECKS, bool)
+
+
+class _PatchSentinel:
+    """Context manager that temporarily flips _DISABLE_DISPOSE_CHECKS."""
+
+    def __init__(self, value: bool):
+        self.value = value
+        self._prev = None
+
+    def __enter__(self):
+        self._prev = _session_mod._DISABLE_DISPOSE_CHECKS
+        _session_mod._DISABLE_DISPOSE_CHECKS = self.value
+        return self
+
+    def __exit__(self, exc_type, exc, tb):
+        _session_mod._DISABLE_DISPOSE_CHECKS = self._prev
+
+
+class TestRequestExecutorEntryGuard(unittest.TestCase):
+    def test_disposed_executor_throws_on_execute_command(self):
+        executor = RequestExecutor.__new__(RequestExecutor)
+        executor._disposed = True
+        with _PatchSentinel(False):
+            with self.assertRaises(RuntimeError) as cm:
+                executor._throw_if_disposed_at_entry()
+        self.assertIn("disposed", str(cm.exception).lower())
+
+    def test_disposed_executor_skipped_when_sentinel_set(self):
+        executor = RequestExecutor.__new__(RequestExecutor)
+        executor._disposed = True
+        with _PatchSentinel(True):
+            # Should NOT raise.
+            executor._throw_if_disposed_at_entry()
+
+    def test_not_disposed_executor_passes(self):
+        executor = RequestExecutor.__new__(RequestExecutor)
+        executor._disposed = False
+        with _PatchSentinel(False):
+            executor._throw_if_disposed_at_entry()
+        with _PatchSentinel(True):
+            executor._throw_if_disposed_at_entry()
+
+
+if __name__ == "__main__":
+    unittest.main()

ravendb/tests/session_tests/test_disposed_guards_integration.py

@@ -0,0 +1,65 @@
+"""
+Integration tests against a live RavenDB 7.2.x server for the 7.2.3
+disposed-guard additions:
+
+  * `session.save_changes()` raises when the parent store has been disposed
+  * The session's own disposed guard fires too
+  * `RAVEN_DISABLE_DISPOSE_CHECKS=true` lets sessions still close cleanly
+    against a disposed store (escape hatch)
+"""
+
+import os
+import unittest
+
+from ravendb.tests.test_base import TestBase, User
+import ravendb.documents.session.document_session_operations.in_memory_document_session_operations as _session_mod
+
+
+class TestDisposedGuardsIntegration(TestBase):
+    def test_save_changes_raises_after_session_close(self):
+        with self.store.open_session() as session:
+            session.store(User("alice"), "users/1")
+            session.save_changes()
+            session.close()
+            with self.assertRaises(RuntimeError) as cm:
+                session.store(User("bob"), "users/2")
+                session.save_changes()
+            self.assertIn("disposed", str(cm.exception).lower())
+
+    def test_save_changes_raises_after_store_dispose(self):
+        # Open a session, close the store, then try to use the session.
+        store = self.get_document_store("test_db_disposed_guard")
+        session = store.open_session()
+        session.store(User("alice"), "users/1")
+        session.save_changes()
+
+        store.close()  # disposes the store
+
+        with self.assertRaises(RuntimeError) as cm:
+            session.save_changes()
+        self.assertIn("disposed", str(cm.exception).lower())
+
+    def test_dispose_check_env_var_disables_store_guard(self):
+        # Same scenario as above but with RAVEN_DISABLE_DISPOSE_CHECKS=true,
+        # the store-disposed half of the guard should be skipped.
+        # We patch the module sentinel directly (the env var is read at
+        # import time, and we already validate that path in the unit tests).
+        prev = _session_mod._DISABLE_DISPOSE_CHECKS
+        _session_mod._DISABLE_DISPOSE_CHECKS = True
+        try:
+            store = self.get_document_store("test_db_disposed_envvar")
+            session = store.open_session()
+            session.store(User("alice"), "users/1")
+            session.save_changes()
+            store.close()
+
+            # The session-level disposed flag still fires when the session
+            # itself is closed. We only verify that calling assert_not_disposed
+            # against a closed store no longer raises.
+            session.assert_not_disposed()
+        finally:
+            _session_mod._DISABLE_DISPOSE_CHECKS = prev
+
+
+if __name__ == "__main__":
+    unittest.main()