oscartbeaumont / rspc

A framework for building typesafe web backends in Rust
https://rspc.dev
MIT License
1.1k stars 50 forks source link

rspc can only handle one request at a time #260

Closed campbellcole closed 3 months ago

campbellcole commented 3 months ago

I don't know if this is considered a bug but I wanted to get clarification on if this is intended. I have a RSPC router that uses a mixture of sync and async procedures. I am trying to implement a system that allows certain procedure calls to require confirmation. The way I've implemented this is by blocking in the procedure body until another procedure call unblocks it.

The issue is that the second procedure call never makes it through because the first one is still waiting. This results in a deadlock.

To me, it doesn't make sense why RSPC would only be able to handle requests sequentially given that RSPC allows async procedures.

I could work around this by spawning another thread/task in the IPC call so it returns immediately, but that would mean I'd lose the return value of the procedure were it to fail. Is there any way to easily lift this restriction, and is this even intended?

Edit: this is in a tauri app btw. It seems like the issue may be in the way the plugin is set up, where it spawns an async task to rx.recv but awaits each request instead of spawning a separate task?

campbellcole commented 3 months ago

Ok I am happy to say that I have discovered the source of this and found a trivial fix. I'll submit a pull request shortly. This issue likely only affects the Tauri plugin.

Edit: never mind about the PR; there's no branch for me to put it. In that case, here is a patch file that will fix this issue in case anybody else encounters it (make sure you git checkout v0.1.3 first):

diff --git a/src/integrations/tauri.rs b/src/integrations/tauri.rs
index 4f6458d..110dcd8 100644
--- a/src/integrations/tauri.rs
+++ b/src/integrations/tauri.rs
@@ -1,10 +1,10 @@
-use std::{collections::HashMap, sync::Arc};
+use std::{borrow::Borrow, collections::HashMap, sync::Arc};

 use tauri::{
     plugin::{Builder, TauriPlugin},
     Manager, Runtime,
 };
-use tokio::sync::mpsc;
+use tokio::sync::{mpsc, Mutex};

 use crate::{
     internal::jsonrpc::{self, handle_json_rpc, Sender, SubscriptionMap},
@@ -22,19 +22,25 @@ where
     Builder::new("rspc")
         .setup(|app_handle| {
             let (tx, mut rx) = mpsc::unbounded_channel::<jsonrpc::Request>();
-            let (mut resp_tx, mut resp_rx) = mpsc::unbounded_channel::<jsonrpc::Response>();
-            let mut subscriptions = HashMap::new();
+            let (resp_tx, mut resp_rx) = mpsc::unbounded_channel::<jsonrpc::Response>();
+            let subscriptions = Arc::new(Mutex::new(HashMap::new()));

             tokio::spawn(async move {
                 while let Some(req) = rx.recv().await {
-                    handle_json_rpc(
-                        ctx_fn(),
-                        req,
-                        &router,
-                        &mut Sender::ResponseChannel(&mut resp_tx),
-                        &mut SubscriptionMap::Ref(&mut subscriptions),
-                    )
-                    .await;
+                    let ctx = ctx_fn();
+                    let router = router.clone();
+                    let mut resp_tx = resp_tx.clone();
+                    let subscriptions = subscriptions.clone();
+                    tokio::spawn(async move {
+                        handle_json_rpc(
+                            ctx,
+                            req,
+                            &router,
+                            &mut Sender::ResponseChannel(&mut resp_tx),
+                            &mut SubscriptionMap::Mutex(subscriptions.borrow()),
+                        )
+                        .await;
+                    });
                 }
             });
oscartbeaumont commented 3 months ago

Okay yeah that will do it. This has been fixed on main for a long time so I completely forgot it was a possibility.

Will push this fix and a couple other things as 0.1.4.