From 22e65782137309749b4fc2b70aae905ea9e93b06 Mon Sep 17 00:00:00 2001 From: Andrew Whitehead Date: Fri, 11 Mar 2022 16:37:47 -0800 Subject: [PATCH 1/5] quietly handle session removal when the related store has been closed Signed-off-by: Andrew Whitehead --- src/ffi/handle.rs | 4 ++-- src/ffi/store.rs | 45 +++++++++++++++++++++++++-------------------- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/ffi/handle.rs b/src/ffi/handle.rs index 8100f614..935ddd12 100644 --- a/src/ffi/handle.rs +++ b/src/ffi/handle.rs @@ -1,4 +1,4 @@ -use std::{marker::PhantomData, mem, sync::Arc}; +use std::{fmt::Display, marker::PhantomData, mem, sync::Arc}; use crate::error::Error; @@ -48,7 +48,7 @@ impl std::fmt::Display for ArcHandle { } } -pub trait ResourceHandle: Copy + Ord + From { +pub trait ResourceHandle: Copy + Ord + From + Display { fn invalid() -> Self { Self::from(0) } diff --git a/src/ffi/store.rs b/src/ffi/store.rs index 9a2a09d6..c8f8cedb 100644 --- a/src/ffi/store.rs +++ b/src/ffi/store.rs @@ -84,17 +84,12 @@ where handle } - pub async fn remove(&self, handle: K) -> Result { - Arc::try_unwrap( - self.map - .write() - .await - .remove(&handle) - .ok_or_else(|| err_msg!("Invalid resource handle"))? - .1, - ) - .map(|item| item.into_inner().unwrap()) - .map_err(|_| err_msg!(Busy, "Resource handle in use")) + pub async fn remove(&self, handle: K) -> Option> { + self.map.write().await.remove(&handle).map(|(_s, v)| { + Arc::try_unwrap(v) + .map(|item| item.into_inner().unwrap()) + .map_err(|_| err_msg!(Busy, "Resource handle in use")) + }) } pub async fn borrow(&self, handle: K) -> Result, Error> { @@ -502,8 +497,13 @@ pub extern "C" fn askar_scan_free(handle: ScanHandle) -> ErrorCode { catch_err! { trace!("Close scan"); spawn_ok(async move { - FFI_SCANS.remove(handle).await.ok(); - info!("Closed scan {}", handle); + // the Scan may have been removed due to the Store being closed + if let Some(scan) = FFI_SCANS.remove(handle).await { + scan.ok(); + info!("Closed scan {}", handle); + } else { + info!("Scan not found for closing: {}", handle); + } }); Ok(ErrorCode::Success) } @@ -996,15 +996,20 @@ pub extern "C" fn askar_session_close( }); spawn_ok(async move { let result = async { - let session = FFI_SESSIONS.remove(handle).await?; - if commit == 0 { - // not necessary - rollback is automatic for txn, - // and for regular session there is no action to perform - // > session.rollback().await?; + // the Session may have been removed due to the Store being closed + if let Some(session) = FFI_SESSIONS.remove(handle).await { + let session = session?; + if commit == 0 { + // not necessary - rollback is automatic for txn, + // and for regular session there is no action to perform + // > session.rollback().await?; + } else { + session.commit().await?; + } + info!("Closed session {}", handle); } else { - session.commit().await?; + info!("Session not found for closing: {}", handle); } - info!("Closed session {}", handle); Ok(()) }.await; if let Some(cb) = cb { From 9879eab8928d83a7b19bd694285380d4112718ff Mon Sep 17 00:00:00 2001 From: Andrew Whitehead Date: Fri, 11 Mar 2022 16:38:06 -0800 Subject: [PATCH 2/5] update to sqlx 0.5.11 Signed-off-by: Andrew Whitehead --- Cargo.toml | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index fbcee068..7db2ba0e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,8 +30,8 @@ any = [] ffi = ["any", "ffi-support", "logger", "option-lock"] jemalloc = ["jemallocator"] logger = ["env_logger", "log"] -postgres = ["sqlx", "sqlx-core", "sqlx/postgres", "sqlx/tls"] -sqlite = ["num_cpus", "sqlx", "sqlx-core", "sqlx/sqlite"] +postgres = ["sqlx", "sqlx/postgres", "sqlx/tls"] +sqlite = ["num_cpus", "sqlx", "sqlx/sqlite"] pg_test = ["postgres"] [dev-dependencies] @@ -72,16 +72,11 @@ path = "./askar-crypto" features = ["all_keys", "any_key", "argon2", "crypto_box", "std"] [dependencies.sqlx] -version = "=0.5.9" +version = "0.5.11" default-features = false features = ["chrono", "runtime-tokio-rustls"] optional = true -[dependencies.sqlx-core] -version = "=0.5.9" -default-features = false -optional = true - [profile.release] lto = true codegen-units = 1 From f2acbecaea6f5dd3a4bb20fc2606e5452455684b Mon Sep 17 00:00:00 2001 From: Andrew Whitehead Date: Fri, 11 Mar 2022 16:39:10 -0800 Subject: [PATCH 3/5] additional tests around profiles Signed-off-by: Andrew Whitehead --- wrappers/python/setup.cfg | 1 + wrappers/python/tests/test_store.py | 40 +++++++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/wrappers/python/setup.cfg b/wrappers/python/setup.cfg index b8ece4a5..0d16027d 100644 --- a/wrappers/python/setup.cfg +++ b/wrappers/python/setup.cfg @@ -10,3 +10,4 @@ per_file_ignores = */__init__.py:D104 minversion = 5.0 testpaths = tests +asyncio_mode=strict diff --git a/wrappers/python/tests/test_store.py b/wrappers/python/tests/test_store.py index da86683c..03ef787e 100644 --- a/wrappers/python/tests/test_store.py +++ b/wrappers/python/tests/test_store.py @@ -1,8 +1,10 @@ import os -from pytest import fixture, mark +from pytest import mark, raises +import pytest_asyncio from aries_askar import ( + AskarError, KeyAlg, Key, Store, @@ -22,7 +24,7 @@ def raw_key() -> str: return Store.generate_raw_key(b"00000000000000000000000000000My1") -@fixture +@pytest_asyncio.fixture async def store() -> Store: key = raw_key() store = await Store.provision(TEST_STORE_URI, "raw", key, recreate=True) @@ -229,4 +231,38 @@ async def test_profile(store: Store): ) == 1 await store_2.close() + with raises(AskarError, match="Duplicate"): + _ = await store.create_profile(profile) + + # check profile is still usable + async with store.session(profile) as session: + assert ( + await session.count( + TEST_ENTRY["category"], {"~plaintag": "a", "enctag": "b"} + ) + ) == 1 + await store.remove_profile(profile) + + # profile key is cached + async with store.session(profile) as session: + assert ( + await session.count( + TEST_ENTRY["category"], {"~plaintag": "a", "enctag": "b"} + ) + ) == 0 + + with raises(AskarError, match="not found"): + async with store.session("unknown profile") as session: + await session.count( + TEST_ENTRY["category"], {"~plaintag": "a", "enctag": "b"} + ) + + await store.create_profile(profile) + + async with store.session(profile) as session: + assert ( + await session.count( + TEST_ENTRY["category"], {"~plaintag": "a", "enctag": "b"} + ) + ) == 0 From f8451a77e171ed8116a196ed0f9fa66160498201 Mon Sep 17 00:00:00 2001 From: Andrew Whitehead Date: Fri, 11 Mar 2022 16:39:44 -0800 Subject: [PATCH 4/5] minor fixes for session close Signed-off-by: Andrew Whitehead --- wrappers/python/aries_askar/bindings.py | 2 +- wrappers/python/aries_askar/store.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/wrappers/python/aries_askar/bindings.py b/wrappers/python/aries_askar/bindings.py index bc92e0b6..85a8b4c7 100644 --- a/wrappers/python/aries_askar/bindings.py +++ b/wrappers/python/aries_askar/bindings.py @@ -67,7 +67,7 @@ def __repr__(self) -> str: async def close(self, commit: bool = False): """Close the session.""" - if not getattr(self, "_closed", False): + if not getattr(self, "_closed", False) and self: await do_call_async( "askar_session_close", self, diff --git a/wrappers/python/aries_askar/store.py b/wrappers/python/aries_askar/store.py index 263f3d3f..d5f86832 100644 --- a/wrappers/python/aries_askar/store.py +++ b/wrappers/python/aries_askar/store.py @@ -569,7 +569,7 @@ async def rollback(self): async def close(self): if self._handle: - await self.handle.close(commit=False) + await self._handle.close(commit=False) self._handle = None def __repr__(self) -> str: From 1852c942bb18c2f4d04a1284f4b734bc2d943471 Mon Sep 17 00:00:00 2001 From: Andrew Whitehead Date: Mon, 14 Mar 2022 11:27:25 -0700 Subject: [PATCH 5/5] run python wrapper tests on python 3.7 to pick up newer pytest-asyncio Signed-off-by: Andrew Whitehead --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 43da2dbf..7eeab8ea 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -205,7 +205,7 @@ jobs: strategy: matrix: os: [ubuntu-latest, macos-11, windows-latest] - python-version: [3.6] + python-version: [3.7] include: - os: ubuntu-latest plat-name: manylinux2014_x86_64