clawhdf5-remote: ObjectStoreStorage works from any thread

It refused whenever Handle::try_current() was Ok, which is also the case
inside spawn_blocking threads — so the workaround its own error message
recommended failed the same way, and the backend could only be used from
a bare std::thread in a tokio application.

Reads are now spawned on the storage's own runtime and the caller waits on
a channel: the future never runs on the caller's thread, so neither a
spawn_blocking thread nor a current-thread runtime can deadlock or panic
(a read inside a runtime blocks that thread, like any blocking call; the
docs still recommend spawn_blocking there).

Tests: a read in spawn_blocking of a multi-thread runtime and a read inside
a current-thread runtime's task give File::open's values (both errors
before).

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

crates/clawhdf5-remote/README.md

@@ -62,11 +62,11 @@ checks both).
 ## Object stores
 
 `object_store` is async; `Storage` is synchronous (parsing is CPU work).
-`ObjectStoreStorage` owns a small tokio runtime (two worker threads) and
-blocks the calling thread on it for each read, so it is read from ordinary
-threads, several at once. From inside an async runtime it refuses
-(`RemoteError::Usage`) rather than block a worker: read in
-`tokio::task::spawn_blocking`. The object is pinned by its ETag
+`ObjectStoreStorage` owns a small tokio runtime (two worker threads): each
+read runs there while the calling thread waits, so it works from any
+thread, several at once — including `tokio::task::spawn_blocking` and code
+inside another runtime (where `spawn_blocking` is still the better place,
+since a read blocks the thread it is called on). The object is pinned by its ETag
 (`If-Match`, and compared on every response), else its version or
 modification time, and its size. The ranges of one read are fetched
 concurrently (up to 8).

crates/clawhdf5-remote/src/object.rs

@@ -3,12 +3,11 @@
 //!
 //! `object_store` is async and [`Storage`] is synchronous (parsing is CPU
 //! work; `docs/design/range-reads.md` §3 (a)). The storage owns a small
-//! multi-threaded tokio runtime (two worker threads) and blocks the calling
-//! thread on it for each read, so it can be used from ordinary threads —
-//! several at once. Calling it from inside another tokio runtime would
-//! block that runtime's worker, so it refuses with
-//! [`RemoteError::Usage`]: from async code, read in
-//! `tokio::task::spawn_blocking`.
+//! multi-threaded tokio runtime (two worker threads): each read is spawned
+//! on it and the calling thread waits for the result, so it can be used
+//! from any thread — several at once, and from async code too. From async
+//! code prefer `tokio::task::spawn_blocking` (a read blocks the thread it
+//! is called on, which inside a runtime is one of its workers).
 //!
 //! The object is pinned when the storage is made: its size, and its ETag
 //! (sent as `If-Match` with every read, and compared with every response)
@@ -105,19 +104,23 @@ impl ObjectStoreStorage {
         )
     }
 
-    fn block_on<T>(
+    /// Run `fut` on the storage's own runtime and wait for it. The future
+    /// never runs on the caller's thread, so the caller's context does not
+    /// matter: a plain thread, `spawn_blocking`, or even inside another
+    /// runtime (whose thread is then blocked for the duration of the read,
+    /// as by any blocking call, but nothing deadlocks or panics).
+    fn block_on<T: Send + 'static>(
         &self,
-        fut: impl std::future::Future<Output = Result<T, RemoteError>>,
+        fut: impl std::future::Future<Output = Result<T, RemoteError>> + Send + 'static,
     ) -> Result<T, RemoteError> {
-        if tokio::runtime::Handle::try_current().is_ok() {
-            return Err(RemoteError::Usage(
-                "ObjectStoreStorage blocks on its own runtime and cannot be read from inside \
-                 an async runtime; read in tokio::task::spawn_blocking"
-                    .into(),
-            ));
-        }
         let rt = self.runtime.as_ref().expect("runtime lives until drop");
-        rt.block_on(fut)
+        let (tx, rx) = std::sync::mpsc::sync_channel(1);
+        rt.spawn(async move {
+            let _ = tx.send(fut.await);
+        });
+        rx.recv().map_err(|_| {
+            RemoteError::ObjectStore("the object store task ended without a result".into())
+        })?
     }
 
     fn options(&self, range: Range<u64>) -> GetOptions {

crates/clawhdf5-remote/tests/object_store.rs

@@ -109,10 +109,51 @@ fn missing_objects_and_async_callers_are_clean_errors() {
     assert!(ObjectStoreStorage::new(store.clone(), ObjectPath::from("nope.h5")).is_err());
     put(store.as_ref(), "m.h5", multi_block_file());
     let storage = ObjectStoreStorage::new(store, ObjectPath::from("m.h5")).unwrap();
-    // From inside a runtime: refused, not a panic or a deadlock.
+    // From inside a current-thread runtime: works (the read runs on the
+    // storage's own runtime), no panic, no deadlock.
     let rt = tokio_rt();
     let r = rt.block_on(async { storage.read_at(0, 10).map(|b| b.len()) });
-    assert!(r.unwrap_err().to_string().contains("spawn_blocking"));
+    assert_eq!(r.unwrap(), 10);
     // Dropping the storage inside a runtime does not panic.
     rt.block_on(async move { drop(storage) });
 }
+
+/// The advice for async callers works: a read in `spawn_blocking` of a
+/// multi-threaded runtime (where `Handle::try_current` is Ok), and a read
+/// straight inside a current-thread runtime's task.
+#[test]
+fn object_stores_read_from_spawn_blocking_and_inside_runtimes() {
+    let store: Arc<dyn ObjectStore> = Arc::new(InMemory::new());
+    let bytes = multi_block_file();
+    put(store.as_ref(), "m.h5", bytes.clone());
+    let want = File::from_bytes(bytes)
+        .unwrap()
+        .dataset("big")
+        .unwrap()
+        .read_f64()
+        .unwrap();
+    let rt = tokio::runtime::Builder::new_multi_thread()
+        .worker_threads(2)
+        .build()
+        .unwrap();
+    let (s, w) = (store.clone(), want.clone());
+    let got = rt
+        .block_on(async move {
+            tokio::task::spawn_blocking(move || {
+                assert!(tokio::runtime::Handle::try_current().is_ok());
+                let (f, _) =
+                    open_object(s, "m.h5", &Options::default()).map_err(|e| e.to_string())?;
+                let v = f.dataset("big").unwrap().read_f64().unwrap();
+                Ok::<_, String>(v == w)
+            })
+            .await
+        })
+        .unwrap();
+    assert_eq!(got, Ok(true));
+    let rt = tokio_rt();
+    let got = rt.block_on(async {
+        let (f, _) = open_object(store, "m.h5", &Options::default()).unwrap();
+        f.dataset("big").unwrap().read_f64().unwrap()
+    });
+    assert_eq!(got, want);
+}