WIP 2

Author: timon-schelling

editor/src/messages/portfolio/resource/mod.rs

@@ -4,4 +4,4 @@ mod resource_message_handler;
 #[doc(inline)]
 pub use resource_message::{ResourceMessage, ResourceMessageDiscriminant};
 #[doc(inline)]
-pub use resource_message_handler::{ResourceMessageContext, ResourceMessageHandler, ResourceReadHandle, resources};
+pub use resource_message_handler::{ResourceMessageContext, ResourceMessageHandler, ResourceReadHandle, resources, set_resource_storage};

editor/src/messages/portfolio/resource/resource_message_handler.rs

@@ -1,21 +1,50 @@
 use crate::messages::prelude::*;
 use graph_craft::application_io::{ResourceFuture, ResourceHash, ResourceStorage, Resources};
-use std::sync::{Arc, RwLock};
+use std::sync::{OnceLock, RwLock};
 
-const RESOURCE_STORAGE: Arc<RwLock<Option<Box<dyn ResourceStorage>>>> = Arc::new(RwLock::new(None));
+/// Process-wide replaceable resource storage. The `RwLock` lets the message handler take exclusive
+/// write access (`&mut dyn ResourceStorage`) when processing a `ResourceMessage`, while any number
+/// of [`ResourceReadHandle`] clones can take the read side concurrently to call `Resources::load`.
+static RESOURCE_STORAGE: OnceLock<RwLock<Option<Box<dyn ResourceStorage>>>> = OnceLock::new();
 
-pub(crate) fn set_resource_storage(storage: Box<dyn ResourceStorage>) {
-	let binding = RESOURCE_STORAGE;
-	let mut guard = binding.write().unwrap();
-	*guard = Some(storage);
+fn cell() -> &'static RwLock<Option<Box<dyn ResourceStorage>>> {
+	RESOURCE_STORAGE.get_or_init(|| RwLock::new(None))
 }
 
-pub fn resources() -> Box<dyn Resources> {
-	let binding = RESOURCE_STORAGE;
-	let guard = binding.read().unwrap();
-	Box::new(ResourceReadHandle {
-		storage: guard.as_ref().expect("Resource storage not initialized").clone(),
-	})
+/// Install (or replace) the underlying storage. Existing `ResourceReadHandle` clones automatically
+/// observe the new backing on their next `load`.
+pub fn set_resource_storage(storage: Box<dyn ResourceStorage>) {
+	*cell().write().unwrap_or_else(|p| p.into_inner()) = Some(storage);
+}
+
+/// Write-side accessor for the message handler only. Returns a write-locked guard that derefs to
+/// `&mut Option<Box<dyn ResourceStorage>>`; private on purpose — the read handle below is the only
+/// surface exposed to the rest of the editor.
+fn resources_mut() -> std::sync::RwLockWriteGuard<'static, Option<Box<dyn ResourceStorage>>> {
+	cell().write().unwrap_or_else(|p| p.into_inner())
+}
+
+/// Unlimited read-only handle. Each `Resources::load` call independently takes the static's read
+/// lock, invokes the backend's `load` (which returns a `'static` owned future), then releases the
+/// lock before awaiting. Crucially the lock is never held across an `.await`, so a concurrent
+/// write/GC from the message handler can never deadlock against a pending IDB fetch.
+pub fn resources() -> ResourceReadHandle {
+	ResourceReadHandle
+}
+
+#[derive(Clone, Copy, Default)]
+pub struct ResourceReadHandle;
+
+impl Resources for ResourceReadHandle {
+	fn load(&self, hash: ResourceHash) -> ResourceFuture {
+		let future = cell().read().unwrap_or_else(|p| p.into_inner()).as_deref().map(|storage| storage.load(hash));
+		Box::pin(async move {
+			match future {
+				Some(future) => future.await,
+				None => None,
+			}
+		})
+	}
 }
 
 #[derive(Default, ExtractField)]
@@ -27,40 +56,35 @@ impl std::fmt::Debug for ResourceMessageHandler {
 	}
 }
 
-/// Read-only access to a `ResourceMessageHandler`'s storage. Cheap to clone.
-#[derive(Clone)]
-pub struct ResourceReadHandle {
-	storage: Arc<RwLock<Box<dyn ResourceStorage>>>,
-}
-
-impl Resources for ResourceReadHandle {
-	fn load<'a>(&'a self, hash: &'a ResourceHash) -> ResourceFuture<'a> {
-		self.storage.load(hash)
-	}
-}
-
 #[derive(ExtractField)]
 pub struct ResourceMessageContext {}
 
 #[message_handler_data]
 impl MessageHandler<ResourceMessage, ResourceMessageContext> for ResourceMessageHandler {
 	fn process_message(&mut self, message: ResourceMessage, responses: &mut VecDeque<Message>, _context: ResourceMessageContext) {
+		let mut guard = resources_mut();
+		let Some(storage) = guard.as_deref_mut() else {
+			log::error!("Resource storage not initialized; dropping {message:?}");
+			return;
+		};
+
 		match message {
 			ResourceMessage::Write { data } => {
-				let _hash = self.storage.write(data.as_ref());
+				let _hash = storage.write(data.as_ref());
 			}
 			ResourceMessage::GarbageCollect { used } => {
-				self.storage.garbage_collect(&used);
+				storage.garbage_collect(&used);
 			}
 			ResourceMessage::Export { document_id, resources } => {
 				let mut exported = Vec::with_capacity(resources.len());
 				for hash in resources.iter() {
-					if let Some(resource) = self.storage.read(hash) {
+					if let Some(resource) = storage.read(hash) {
 						exported.push((*hash, resource));
 					} else {
 						log::error!("Resource not found for hash {hash} during export");
 					}
 				}
+				drop(guard);
 
 				responses.add(PortfolioMessage::DocumentPassMessage {
 					document_id,

node-graph/graph-craft/src/application_io.rs

@@ -60,7 +60,7 @@ impl ApplicationIo for PlatformApplicationIo {
 		self.gpu_executor.as_ref()
 	}
 
-	fn load_resource<'a>(&'a self, hash: &'a ResourceHash) -> graphene_application_io::ResourceFuture<'a> {
+	fn load_resource(&self, hash: ResourceHash) -> graphene_application_io::ResourceFuture {
 		self.resources.as_ref().expect("Resource storage not initialized").load(hash)
 	}
 }

node-graph/graph-craft/src/application_io/resource.rs

@@ -3,7 +3,7 @@ pub mod indexed_db;
 #[cfg(not(target_family = "wasm"))]
 pub mod mmap;
 
-use graphene_application_io::{Resource, ResourceFuture, ResourceHash, Resources, ResourceStorage};
+use graphene_application_io::{Resource, ResourceFuture, ResourceHash, ResourceStorage, Resources};
 use std::collections::HashMap;
 use std::sync::{Arc, Mutex};
 
@@ -19,29 +19,29 @@ impl HashMapResourceStorage {
 }
 
 impl Resources for HashMapResourceStorage {
-	fn load<'a>(&'a self, hash: &'a ResourceHash) -> ResourceFuture<'a> {
-		let result = ResourceStorage::read(self, hash);
+	fn load(&self, hash: ResourceHash) -> ResourceFuture {
+		let result = self.resources.lock().unwrap().get(&hash).cloned();
 		Box::pin(async move { result })
 	}
 }
 
 impl ResourceStorage for HashMapResourceStorage {
-	fn read(&self, hash: &ResourceHash) -> Option<Resource> {
-		self.resources.lock().unwrap().get(hash).cloned()
+	fn read(&mut self, hash: &ResourceHash) -> Option<Resource> {
+		self.resources.get_mut().unwrap().get(hash).cloned()
 	}
 
-	fn write(&self, data: &[u8]) -> ResourceHash {
+	fn write(&mut self, data: &[u8]) -> ResourceHash {
 		let hash = ResourceHash::from(data);
-		self.resources.lock().unwrap().insert(hash, Resource::new(Arc::<[u8]>::from(data)));
+		self.resources.get_mut().unwrap().insert(hash, Resource::new(Arc::<[u8]>::from(data)));
 		hash
 	}
 
-	fn contains(&self, hash: &ResourceHash) -> bool {
-		self.resources.lock().unwrap().contains_key(hash)
+	fn contains(&mut self, hash: &ResourceHash) -> bool {
+		self.resources.get_mut().unwrap().contains_key(hash)
 	}
 
-	fn garbage_collect(&self, used: &[ResourceHash]) {
+	fn garbage_collect(&mut self, used: &[ResourceHash]) {
 		let used_set: std::collections::HashSet<&ResourceHash> = used.iter().collect();
-		self.resources.lock().unwrap().retain(|hash, _| used_set.contains(hash));
+		self.resources.get_mut().unwrap().retain(|hash, _| used_set.contains(hash));
 	}
 }

node-graph/graph-craft/src/application_io/resource/indexed_db.rs

@@ -133,33 +133,41 @@ impl IndexedDbResourceStorage {
 }
 
 impl Resources for IndexedDbResourceStorage {
-	fn load<'a>(&'a self, hash: &'a ResourceHash) -> ResourceFuture<'a> {
-		// The underlying future captures `&IdbDatabase` which is not `Send`, but wasm is single
-		// threaded so the trait's `Send` bound is purely structural here; wrap it accordingly.
+	fn load(&self, hash: ResourceHash) -> ResourceFuture {
+		// Clone the internal `Arc`s out so the returned future owns its state and doesn't borrow
+		// `self`. This lets the read-side handle drop its `RwLock` read guard before awaiting, which
+		// is what avoids the otherwise-fatal write-vs-read deadlock on single-threaded wasm.
+		let db = self.db.clone();
+		let store_name = self.store_name.clone();
+		let cache = self.cache.clone();
+		let known_keys = self.known_keys.clone();
+
+		// The IDB future captures non-`Send` JS handles; wasm has a single thread so re-asserting
+		// `Send` via `UnsafeSendFuture` is sound here.
 		Box::pin(UnsafeSendFuture(async move {
-			if let Some(resource) = self.cache.lock().unwrap().get(hash) {
+			if let Some(resource) = cache.lock().unwrap().get(&hash) {
 				return Some(resource.clone());
 			}
-			if !self.known_keys.lock().unwrap().contains(hash) {
+			if !known_keys.lock().unwrap().contains(&hash) {
 				return None;
 			}
 
-			// Cache miss for a known key: fetch from IndexedDB and wait for it to land before returning.
-			match fetch_one(&self.db, &self.store_name, *hash).await {
+			// Cache miss for a known key: fetch from IndexedDB and wait for it to land.
+			match fetch_one(&db, &store_name, hash).await {
 				Ok(Some(payload)) => {
 					let recomputed = ResourceHash::from(payload.as_slice());
-					if recomputed != *hash {
+					if recomputed != hash {
 						log::warn!("IndexedDB entry's hash does not match its payload: {hash} vs {recomputed}");
-						self.known_keys.lock().unwrap().remove(hash);
+						known_keys.lock().unwrap().remove(&hash);
 						None
 					} else {
 						let resource = Resource::new(Arc::<[u8]>::from(payload));
-						self.cache.lock().unwrap().insert(*hash, resource.clone());
+						cache.lock().unwrap().insert(hash, resource.clone());
 						Some(resource)
 					}
 				}
 				Ok(None) => {
-					self.known_keys.lock().unwrap().remove(hash);
+					known_keys.lock().unwrap().remove(&hash);
 					None
 				}
 				Err(error) => {
@@ -172,24 +180,24 @@ impl Resources for IndexedDbResourceStorage {
 }
 
 impl ResourceStorage for IndexedDbResourceStorage {
-	fn read(&self, hash: &ResourceHash) -> Option<Resource> {
+	fn read(&mut self, hash: &ResourceHash) -> Option<Resource> {
 		// Cache-only lookup; sync callers (e.g. document export) take what's already hydrated.
 		self.cache.lock().unwrap().get(hash).cloned()
 	}
 
-	fn write(&self, data: &[u8]) -> ResourceHash {
+	fn write(&mut self, data: &[u8]) -> ResourceHash {
 		let hash = ResourceHash::from(data);
 		self.cache.lock().unwrap().insert(hash, Resource::new(Arc::<[u8]>::from(data)));
 		self.known_keys.lock().unwrap().insert(hash);
 		self.enqueue_put(hash, data.to_vec());
 		hash
 	}
 
-	fn contains(&self, hash: &ResourceHash) -> bool {
+	fn contains(&mut self, hash: &ResourceHash) -> bool {
 		self.cache.lock().unwrap().contains_key(hash) || self.known_keys.lock().unwrap().contains(hash)
 	}
 
-	fn garbage_collect(&self, used: &[ResourceHash]) {
+	fn garbage_collect(&mut self, used: &[ResourceHash]) {
 		let used_set: std::collections::HashSet<ResourceHash> = used.iter().cloned().collect();
 
 		let to_delete: Vec<ResourceHash> = {

node-graph/graph-craft/src/application_io/resource/mmap.rs

@@ -48,28 +48,29 @@ impl MmapResourceStorage {
 }
 
 impl Resources for MmapResourceStorage {
-	fn load<'a>(&'a self, hash: &'a ResourceHash) -> ResourceFuture<'a> {
-		let result = ResourceStorage::read(self, hash);
+	fn load(&self, hash: ResourceHash) -> ResourceFuture {
+		// Cache-only lookup; the on-demand mmap path lives in `ResourceStorage::read` and is reachable
+		// only through the message handler's mut access.
+		let result = self.cache.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).get(&hash).cloned();
 		Box::pin(async move { result })
 	}
 }
 
 impl ResourceStorage for MmapResourceStorage {
-	fn read(&self, hash: &ResourceHash) -> Option<Resource> {
-		let mut cache = self.cache.lock().unwrap_or_else(|poisoned| poisoned.into_inner());
-		if let Some(resource) = cache.get(hash) {
+	fn read(&mut self, hash: &ResourceHash) -> Option<Resource> {
+		if let Some(resource) = self.cache.get_mut().unwrap_or_else(|poisoned| poisoned.into_inner()).get(hash) {
 			return Some(resource.clone());
 		}
 
 		let path = self.path_for(hash);
 		let mmap = Self::open_mmap(&path)?;
 		let resource = Resource::new(MmappedBytes(mmap));
 
-		cache.insert(*hash, resource.clone());
+		self.cache.get_mut().unwrap_or_else(|poisoned| poisoned.into_inner()).insert(*hash, resource.clone());
 		Some(resource)
 	}
 
-	fn write(&self, data: &[u8]) -> ResourceHash {
+	fn write(&mut self, data: &[u8]) -> ResourceHash {
 		let hash = ResourceHash::from(data);
 		let path = self.path_for(&hash);
 
@@ -108,13 +109,13 @@ impl ResourceStorage for MmapResourceStorage {
 		hash
 	}
 
-	fn contains(&self, hash: &ResourceHash) -> bool {
-		self.cache.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).contains_key(hash) || self.path_for(hash).exists()
+	fn contains(&mut self, hash: &ResourceHash) -> bool {
+		self.cache.get_mut().unwrap_or_else(|poisoned| poisoned.into_inner()).contains_key(hash) || self.path_for(hash).exists()
 	}
 
-	fn garbage_collect(&self, used: &[ResourceHash]) {
+	fn garbage_collect(&mut self, used: &[ResourceHash]) {
 		let used_set: std::collections::HashSet<ResourceHash> = used.iter().cloned().collect();
-		self.cache.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).retain(|hash, _| used_set.contains(hash));
+		self.cache.get_mut().unwrap_or_else(|poisoned| poisoned.into_inner()).retain(|hash, _| used_set.contains(hash));
 
 		let Ok(top_entries) = fs::read_dir(&self.root) else { return };
 		for top_entry in top_entries.flatten() {

node-graph/libraries/application-io/src/lib.rs

@@ -48,7 +48,7 @@ pub trait ApplicationIo {
 	fn gpu_executor(&self) -> Option<&Self::Executor> {
 		None
 	}
-	fn load_resource<'a>(&'a self, hash: &'a ResourceHash) -> resource::ResourceFuture<'a>;
+	fn load_resource(&self, hash: ResourceHash) -> resource::ResourceFuture;
 }
 
 impl<T: ApplicationIo> ApplicationIo for &T {
@@ -58,7 +58,7 @@ impl<T: ApplicationIo> ApplicationIo for &T {
 		(**self).gpu_executor()
 	}
 
-	fn load_resource<'a>(&'a self, hash: &'a ResourceHash) -> resource::ResourceFuture<'a> {
+	fn load_resource(&self, hash: ResourceHash) -> resource::ResourceFuture {
 		(**self).load_resource(hash)
 	}
 }

node-graph/libraries/application-io/src/resource.rs

@@ -165,13 +165,19 @@ unsafe impl StaticType for Resource {
 	type Static = Resource;
 }
 
-pub type ResourceFuture<'a> = Pin<Box<dyn Future<Output = Option<Resource>> + Send + 'a>>;
+/// Owned (`'static`) future returned by [`Resources::load`]. Ownership is what allows the read-side
+/// handle to drop its `RwLock` read guard before awaiting — backends must not borrow their storage
+/// across awaits, instead cloning out any internal `Arc`s they need.
+pub type ResourceFuture = Pin<Box<dyn Future<Output = Option<Resource>> + Send + 'static>>;
 
 pub trait Resources: Send + Sync {
-	fn load<'a>(&'a self, hash: &'a ResourceHash) -> ResourceFuture<'a>;
+	fn load(&self, hash: ResourceHash) -> ResourceFuture;
 }
 
 pub trait ResourceStorage: Resources {
+	// Methods take `&mut self` so only the unique mutable owner (the resource message handler,
+	// guarded behind the static `RwLock`'s write lock) can touch the write surface; the `Resources`
+	// supertrait remains accessible through a shared `&self` borrow for any number of read handles.
 	fn read(&mut self, hash: &ResourceHash) -> Option<Resource>;
 	fn write(&mut self, data: &[u8]) -> ResourceHash;
 	fn contains(&mut self, hash: &ResourceHash) -> bool;

node-graph/nodes/gstd/src/platform_application_io.rs

@@ -264,7 +264,7 @@ where
 #[node_macro::node(category(""))]
 pub async fn resource<'a: 'n>(_: impl Ctx, _primary: (), #[scope("editor-api")] editor_api: &'a PlatformEditorApi, hash: ResourceHash) -> Resource {
 	let application_io = editor_api.application_io.as_ref().expect("ApplicationIo must be available when using resources");
-	application_io.load_resource(&hash).await.unwrap_or_else(|| {
+	application_io.load_resource(hash).await.unwrap_or_else(|| {
 		panic!("Resource {hash} not found");
 	})
 }