Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rspc can only handle one request at a time #260

Closed
campbellcole opened this issue Mar 16, 2024 · 2 comments
Closed

rspc can only handle one request at a time #260

campbellcole opened this issue Mar 16, 2024 · 2 comments

Comments

@campbellcole
Copy link

campbellcole commented Mar 16, 2024

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
Copy link
Author

campbellcole commented Mar 16, 2024

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 added a commit that referenced this issue Mar 17, 2024
Co-authored-by: campbellcole <10430178+campbellcole@users.noreply.github.com>
@oscartbeaumont
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants