From 2314fcb57e8b132d1c2070f466a4f07a3f642256 Mon Sep 17 00:00:00 2001 From: John Smith Date: Sun, 18 Jun 2023 20:57:51 -0400 Subject: [PATCH 1/2] more release lifetime cleanup and base class contextmanager stuff --- .vscode/settings.json | 5 +- veilid-core/src/storage_manager/mod.rs | 23 +++++--- veilid-core/src/table_store/table_store.rs | 2 +- veilid-python/tests/test_dht.py | 45 ++++++++++++---- veilid-python/veilid/api.py | 61 ++++++++++++++++++++++ veilid-python/veilid/json_api.py | 50 ++++++------------ 6 files changed, 133 insertions(+), 53 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index cad7657d..d0656d37 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,3 +1,6 @@ { - "cmake.configureOnOpen": false + "cmake.configureOnOpen": false, + "python.analysis.extraPaths": [ + "veilid-python/.venv/lib/python3.11/site-packages" + ], } \ No newline at end of file diff --git a/veilid-core/src/storage_manager/mod.rs b/veilid-core/src/storage_manager/mod.rs index 64a42530..dbef0bc8 100644 --- a/veilid-core/src/storage_manager/mod.rs +++ b/veilid-core/src/storage_manager/mod.rs @@ -233,8 +233,11 @@ impl StorageManager { force_refresh: bool, ) -> VeilidAPIResult> { let mut inner = self.lock().await?; - let Some(opened_record) = inner.opened_records.remove(&key) else { - apibail_generic!("record not open"); + let safety_selection = { + let Some(opened_record) = inner.opened_records.get(&key) else { + apibail_generic!("record not open"); + }; + opened_record.safety_selection() }; // See if the requested subkey is our local record store @@ -269,7 +272,7 @@ impl StorageManager { rpc_processor, key, subkey, - opened_record.safety_selection(), + safety_selection, last_subkey_result, ) .await?; @@ -307,12 +310,18 @@ impl StorageManager { apibail_generic!("unsupported cryptosystem"); }; - let Some(opened_record) = inner.opened_records.remove(&key) else { - apibail_generic!("record not open"); + let (safety_selection, opt_writer) = { + let Some(opened_record) = inner.opened_records.get(&key) else { + apibail_generic!("record not open"); + }; + ( + opened_record.safety_selection(), + opened_record.writer().cloned(), + ) }; // If we don't have a writer then we can't write - let Some(writer) = opened_record.writer().cloned() else { + let Some(writer) = opt_writer else { apibail_generic!("value is not writable"); }; @@ -371,7 +380,7 @@ impl StorageManager { rpc_processor, key, subkey, - opened_record.safety_selection(), + safety_selection, signed_value_data, descriptor, ) diff --git a/veilid-core/src/table_store/table_store.rs b/veilid-core/src/table_store/table_store.rs index 1831b46e..9fd85f63 100644 --- a/veilid-core/src/table_store/table_store.rs +++ b/veilid-core/src/table_store/table_store.rs @@ -428,7 +428,7 @@ impl TableStore { } pub(crate) fn on_table_db_drop(&self, table: String) { - log_rtab!(debug "dropping table db: {}", table); + log_rtab!("dropping table db: {}", table); let mut inner = self.inner.lock(); if inner.opened.remove(&table).is_none() { unreachable!("should have removed an item"); diff --git a/veilid-python/tests/test_dht.py b/veilid-python/tests/test_dht.py index 35c49e1b..d7f196fb 100644 --- a/veilid-python/tests/test_dht.py +++ b/veilid-python/tests/test_dht.py @@ -1,13 +1,13 @@ -# Routing context veilid tests +# # Routing context veilid tests -import veilid -import pytest -import asyncio -import json -from . import * +# import veilid +# import pytest +# import asyncio +# import json +# from . import * -################################################################## -BOGUS_KEY = veilid.TypedKey.from_value(veilid.CryptoKind.CRYPTO_KIND_VLD0, veilid.PublicKey.from_bytes(b' ')) +# ################################################################## +# BOGUS_KEY = veilid.TypedKey.from_value(veilid.CryptoKind.CRYPTO_KIND_VLD0, veilid.PublicKey.from_bytes(b' ')) # @pytest.mark.asyncio # async def test_get_dht_value_unopened(api_connection: veilid.VeilidAPI): @@ -46,6 +46,29 @@ BOGUS_KEY = veilid.TypedKey.from_value(veilid.CryptoKind.CRYPTO_KIND_VLD0, veili # await rc.close_dht_record(rec.key) # await rc.delete_dht_record(rec.key) -# xxx make tests for tabledb api first -# xxx then make a test that creates a record, stores it in a table -# xxx then make another test that gets the keys from the table and closes/deletes them +# @pytest.mark.asyncio +# async def test_get_dht_value_nonexistent(api_connection: veilid.VeilidAPI): +# rc = await api_connection.new_routing_context() +# async with rc: +# rec = await rc.create_dht_record(veilid.CryptoKind.CRYPTO_KIND_VLD0, veilid.DHTSchema.dflt(1)) +# assert await rc.get_dht_value(rec.key, 0, False) == None +# await rc.close_dht_record(rec.key) +# await rc.delete_dht_record(rec.key) + +# @pytest.mark.asyncio +# async def test_set_get_dht_value(api_connection: veilid.VeilidAPI): +# rc = await api_connection.new_routing_context() +# async with rc: +# rec = await rc.create_dht_record(veilid.CryptoKind.CRYPTO_KIND_VLD0, veilid.DHTSchema.dflt(1)) + +# vd = await rc.set_dht_value(rec.key, 0, b"BLAH BLAH BLAH") +# assert vd != None + +# vd2 = await rc.get_dht_value(rec.key, 0, False) +# assert vd2 != None + +# assert vd == vd2 + +# await rc.close_dht_record(rec.key) +# await rc.delete_dht_record(rec.key) + diff --git a/veilid-python/veilid/api.py b/veilid-python/veilid/api.py index 28ef4b1b..e468eb6d 100644 --- a/veilid-python/veilid/api.py +++ b/veilid-python/veilid/api.py @@ -6,6 +6,18 @@ from .state import VeilidState class RoutingContext(ABC): + + async def __aenter__(self) -> Self: + return self + + async def __aexit__(self, *excinfo): + if not self.is_done(): + await self.release() + + @abstractmethod + def is_done(self) -> bool: + pass + @abstractmethod async def release(self): pass @@ -84,6 +96,17 @@ class RoutingContext(ABC): class TableDbTransaction(ABC): + async def __aenter__(self) -> Self: + return self + + async def __aexit__(self, *excinfo): + if not self.is_done(): + await self.rollback() + + @abstractmethod + def is_done(self) -> bool: + pass + @abstractmethod async def commit(self): pass @@ -102,6 +125,17 @@ class TableDbTransaction(ABC): class TableDb(ABC): + async def __aenter__(self) -> Self: + return self + + async def __aexit__(self, *excinfo): + if not self.is_done(): + await self.release() + + @abstractmethod + def is_done(self) -> bool: + pass + @abstractmethod async def release(self): pass @@ -132,6 +166,18 @@ class TableDb(ABC): class CryptoSystem(ABC): + + async def __aenter__(self) -> Self: + return self + + async def __aexit__(self, *excinfo): + if not self.is_done(): + await self.release() + + @abstractmethod + def is_done(self) -> bool: + pass + @abstractmethod async def release(self): pass @@ -246,6 +292,21 @@ class CryptoSystem(ABC): class VeilidAPI(ABC): + async def __aenter__(self) -> Self: + return self + + async def __aexit__(self, *excinfo): + if not self.is_done(): + await self.release() + + @abstractmethod + def is_done(self) -> bool: + pass + + @abstractmethod + async def release(self): + pass + @abstractmethod async def control(self, args: list[str]) -> str: pass diff --git a/veilid-python/veilid/json_api.py b/veilid-python/veilid/json_api.py index a072e367..6881395a 100644 --- a/veilid-python/veilid/json_api.py +++ b/veilid-python/veilid/json_api.py @@ -53,7 +53,8 @@ class _JsonVeilidAPI(VeilidAPI): writer: Optional[asyncio.StreamWriter] update_callback: Callable[[VeilidUpdate], Awaitable] handle_recv_messages_task: Optional[asyncio.Task] - validate_schemas: bool + validate_schema: bool + done: bool # Shared Mutable State lock: asyncio.Lock next_id: int @@ -70,17 +71,12 @@ class _JsonVeilidAPI(VeilidAPI): self.writer = writer self.update_callback = update_callback self.validate_schema = validate_schema + self.done = False self.handle_recv_messages_task = None self.lock = asyncio.Lock() self.next_id = 1 self.in_flight_requests = dict() - async def __aenter__(self) -> Self: - return self - - async def __aexit__(self, *excinfo): - await self.close() - async def _cleanup_close(self): await self.lock.acquire() try: @@ -96,7 +92,10 @@ class _JsonVeilidAPI(VeilidAPI): finally: self.lock.release() - async def close(self): + def is_done(self) -> bool: + return self.done + + async def release(self): # Take the task await self.lock.acquire() try: @@ -112,6 +111,7 @@ class _JsonVeilidAPI(VeilidAPI): await handle_recv_messages_task except asyncio.CancelledError: pass + self.done = True @classmethod async def connect( @@ -430,12 +430,8 @@ class _JsonRoutingContext(RoutingContext): # complain raise AssertionError("Should have released routing context before dropping object") - async def __aenter__(self) -> Self: - return self - - async def __aexit__(self, *excinfo): - if not self.done: - await self.release() + def is_done(self) -> bool: + return self.done async def release(self): if self.done: @@ -668,12 +664,8 @@ class _JsonTableDbTransaction(TableDbTransaction): # complain raise AssertionError("Should have committed or rolled back transaction before dropping object") - async def __aenter__(self) -> Self: - return self - - async def __aexit__(self, *excinfo): - if not self.done: - await self.rollback() + def is_done(self) -> bool: + return self.done async def commit(self): if self.done: @@ -753,12 +745,8 @@ class _JsonTableDb(TableDb): # complain raise AssertionError("Should have released table db before dropping object") - async def __aenter__(self) -> Self: - return self - - async def __aexit__(self, *excinfo): - if not self.done: - await self.release() + def is_done(self) -> bool: + return self.done async def release(self): if self.done: @@ -880,13 +868,9 @@ class _JsonCryptoSystem(CryptoSystem): # complain raise AssertionError("Should have released crypto system before dropping object") - async def __aenter__(self) -> Self: - return self - - async def __aexit__(self, *excinfo): - if not self.done: - await self.release() - + def is_done(self) -> bool: + return self.done + async def release(self): if self.done: return From e2824da06ed57595730d90297bd63f35650c5d01 Mon Sep 17 00:00:00 2001 From: John Smith Date: Sun, 18 Jun 2023 21:34:40 -0400 Subject: [PATCH 2/2] another tabledb fix --- veilid-core/src/table_store/table_store.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/veilid-core/src/table_store/table_store.rs b/veilid-core/src/table_store/table_store.rs index 9fd85f63..fb293c44 100644 --- a/veilid-core/src/table_store/table_store.rs +++ b/veilid-core/src/table_store/table_store.rs @@ -557,15 +557,15 @@ impl TableStore { let deleted = self.table_store_driver.delete(&table_name).await?; if !deleted { // Table missing? Just remove name - self.name_delete(&name) - .await - .expect("failed to delete name"); warn!( "table existed in name table but not in storage: {} : {}", name, table_name ); - return Ok(false); } + self.name_delete(&name) + .await + .expect("failed to delete name"); + self.flush().await; Ok(true) } @@ -581,6 +581,8 @@ impl TableStore { } } trace!("TableStore::rename {} -> {}", old_name, new_name); - self.name_rename(old_name, new_name).await + self.name_rename(old_name, new_name).await?; + self.flush().await; + Ok(()) } }