Repository: 1c3t3a/rust-socketio (language: rust)

Issue:
`ClientBuilder::on(_any)` should be able to take `!Sync` functions too
There's no reason to keep callbacks under `std::sync::RwLock` if it's `FnMut`, so exclusive access is needed anyways. `std::sync::Mutex` should be used instead as it implements `Sync` regardles if the wrapped value implements `Sync` or not.


Discussion / hints:
Sounds like a fair assessment, thanks for the issue. Do you mind submit a PR for that?

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index 676b2128..560ef7ca 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1379,7 +1379,7 @@ dependencies = [
 
 [[package]]
 name = "rust_engineio"
-version = "0.3.1"
+version = "0.4.0"
 dependencies = [
  "adler32",
  "async-stream",
diff --git a/socketio/src/client/builder.rs b/socketio/src/client/builder.rs
index 3d2f3b56..10eb8797 100644
--- a/socketio/src/client/builder.rs
+++ b/socketio/src/client/builder.rs
@@ -10,7 +10,7 @@ use url::Url;
 use crate::client::callback::{SocketAnyCallback, SocketCallback};
 use crate::error::Result;
 use std::collections::HashMap;
-use std::sync::{Arc, RwLock};
+use std::sync::{Arc, Mutex};
 
 use crate::socket::Socket as InnerSocket;
 
@@ -34,8 +34,8 @@ pub enum TransportType {
 #[derive(Clone)]
 pub struct ClientBuilder {
     address: String,
-    on: Arc<RwLock<HashMap<Event, Callback<SocketCallback>>>>,
-    on_any: Arc<RwLock<Option<Callback<SocketAnyCallback>>>>,
+    on: Arc<Mutex<HashMap<Event, Callback<SocketCallback>>>>,
+    on_any: Arc<Mutex<Option<Callback<SocketAnyCallback>>>>,
     namespace: String,
     tls_config: Option<TlsConnector>,
     opening_headers: Option<HeaderMap>,
@@ -82,8 +82,8 @@ impl ClientBuilder {
     pub fn new<T: Into<String>>(address: T) -> Self {
         Self {
             address: address.into(),
-            on: Arc::new(RwLock::new(HashMap::new())),
-            on_any: Arc::new(RwLock::new(None)),
+            on: Arc::new(Mutex::new(HashMap::new())),
+            on_any: Arc::new(Mutex::new(None)),
             namespace: "/".to_owned(),
             tls_config: None,
             opening_headers: None,
@@ -149,11 +149,11 @@ impl ClientBuilder {
     #[allow(unused_mut)]
     pub fn on<T: Into<Event>, F>(mut self, event: T, callback: F) -> Self
     where
-        F: for<'a> FnMut(Payload, RawClient) + 'static + Sync + Send,
+        F: FnMut(Payload, RawClient) + 'static + Send,
     {
         let callback = Callback::<SocketCallback>::new(callback);
         // SAFETY: Lock is held for such amount of time no code paths lead to a panic while lock is held
-        self.on.write().unwrap().insert(event.into(), callback);
+        self.on.lock().unwrap().insert(event.into(), callback);
         self
     }
 
@@ -177,11 +177,11 @@ impl ClientBuilder {
     #[allow(unused_mut)]
     pub fn on_any<F>(mut self, callback: F) -> Self
     where
-        F: for<'a> FnMut(Event, Payload, RawClient) + 'static + Sync + Send,
+        F: FnMut(Event, Payload, RawClient) + 'static + Send,
     {
         let callback = Some(Callback::<SocketAnyCallback>::new(callback));
         // SAFETY: Lock is held for such amount of time no code paths lead to a panic while lock is held
-        *self.on_any.write().unwrap() = callback;
+        *self.on_any.lock().unwrap() = callback;
         self
     }
 
diff --git a/socketio/src/client/callback.rs b/socketio/src/client/callback.rs
index 4449d834..1015ec03 100644
--- a/socketio/src/client/callback.rs
+++ b/socketio/src/client/callback.rs
@@ -6,9 +6,8 @@ use std::{
 use super::RawClient;
 use crate::{Event, Payload};
 
-pub(crate) type SocketCallback = Box<dyn for<'a> FnMut(Payload, RawClient) + 'static + Sync + Send>;
-pub(crate) type SocketAnyCallback =
-    Box<dyn for<'a> FnMut(Event, Payload, RawClient) + 'static + Sync + Send>;
+pub(crate) type SocketCallback = Box<dyn FnMut(Payload, RawClient) + 'static + Send>;
+pub(crate) type SocketAnyCallback = Box<dyn FnMut(Event, Payload, RawClient) + 'static + Send>;
 
 pub(crate) struct Callback<T> {
     inner: T,
@@ -23,7 +22,7 @@ impl Debug for Callback<SocketCallback> {
 }
 
 impl Deref for Callback<SocketCallback> {
-    type Target = dyn for<'a> FnMut(Payload, RawClient) + 'static + Sync + Send;
+    type Target = dyn FnMut(Payload, RawClient) + 'static + Send;
 
     fn deref(&self) -> &Self::Target {
         self.inner.as_ref()
@@ -39,7 +38,7 @@ impl DerefMut for Callback<SocketCallback> {
 impl Callback<SocketCallback> {
     pub(crate) fn new<T>(callback: T) -> Self
     where
-        T: for<'a> FnMut(Payload, RawClient) + 'static + Sync + Send,
+        T: FnMut(Payload, RawClient) + 'static + Send,
     {
         Callback {
             inner: Box::new(callback),
@@ -56,7 +55,7 @@ impl Debug for Callback<SocketAnyCallback> {
 }
 
 impl Deref for Callback<SocketAnyCallback> {
-    type Target = dyn for<'a> FnMut(Event, Payload, RawClient) + 'static + Sync + Send;
+    type Target = dyn FnMut(Event, Payload, RawClient) + 'static + Send;
 
     fn deref(&self) -> &Self::Target {
         self.inner.as_ref()
@@ -72,7 +71,7 @@ impl DerefMut for Callback<SocketAnyCallback> {
 impl Callback<SocketAnyCallback> {
     pub(crate) fn new<T>(callback: T) -> Self
     where
-        T: for<'a> FnMut(Event, Payload, RawClient) + 'static + Sync + Send,
+        T: FnMut(Event, Payload, RawClient) + 'static + Send,
     {
         Callback {
             inner: Box::new(callback),
diff --git a/socketio/src/client/client.rs b/socketio/src/client/client.rs
index 0921d2ec..5e38c85e 100644
--- a/socketio/src/client/client.rs
+++ b/socketio/src/client/client.rs
@@ -113,7 +113,7 @@ impl Client {
         callback: F,
     ) -> Result<()>
     where
-        F: for<'a> FnMut(Payload, RawClient) + 'static + Sync + Send,
+        F: FnMut(Payload, RawClient) + 'static + Send,
         E: Into<Event>,
         D: Into<Payload>,
     {
diff --git a/socketio/src/client/raw_client.rs b/socketio/src/client/raw_client.rs
index 8b1dcda1..3f9a6bae 100644
--- a/socketio/src/client/raw_client.rs
+++ b/socketio/src/client/raw_client.rs
@@ -7,7 +7,7 @@ use crate::client::callback::{SocketAnyCallback, SocketCallback};
 use crate::error::Result;
 use std::collections::HashMap;
 use std::ops::DerefMut;
-use std::sync::{Arc, RwLock};
+use std::sync::{Arc, Mutex};
 use std::time::Duration;
 use std::time::Instant;
 
@@ -32,9 +32,9 @@ pub struct Ack {
 pub struct RawClient {
     /// The inner socket client to delegate the methods to.
     socket: InnerSocket,
-    on: Arc<RwLock<HashMap<Event, Callback<SocketCallback>>>>,
-    on_any: Arc<RwLock<Option<Callback<SocketAnyCallback>>>>,
-    outstanding_acks: Arc<RwLock<Vec<Ack>>>,
+    on: Arc<Mutex<HashMap<Event, Callback<SocketCallback>>>>,
+    on_any: Arc<Mutex<Option<Callback<SocketAnyCallback>>>>,
+    outstanding_acks: Arc<Mutex<Vec<Ack>>>,
     // namespace, for multiplexing messages
     nsp: String,
     // Data sent in opening header
@@ -49,8 +49,8 @@ impl RawClient {
     pub(crate) fn new<T: Into<String>>(
         socket: InnerSocket,
         namespace: T,
-        on: Arc<RwLock<HashMap<Event, Callback<SocketCallback>>>>,
-        on_any: Arc<RwLock<Option<Callback<SocketAnyCallback>>>>,
+        on: Arc<Mutex<HashMap<Event, Callback<SocketCallback>>>>,
+        on_any: Arc<Mutex<Option<Callback<SocketAnyCallback>>>>,
         auth: Option<serde_json::Value>,
     ) -> Result<Self> {
         Ok(RawClient {
@@ -58,7 +58,7 @@ impl RawClient {
             nsp: namespace.into(),
             on,
             on_any,
-            outstanding_acks: Arc::new(RwLock::new(Vec::new())),
+            outstanding_acks: Arc::new(Mutex::new(Vec::new())),
             auth,
         })
     }
@@ -195,7 +195,7 @@ impl RawClient {
         callback: F,
     ) -> Result<()>
     where
-        F: for<'a> FnMut(Payload, RawClient) + 'static + Sync + Send,
+        F: FnMut(Payload, RawClient) + 'static + Send,
         E: Into<Event>,
         D: Into<Payload>,
     {
@@ -212,7 +212,7 @@ impl RawClient {
         };
 
         // add the ack to the tuple of outstanding acks
-        self.outstanding_acks.write()?.push(ack);
+        self.outstanding_acks.lock()?.push(ack);
 
         self.socket.send(socket_packet)?;
         Ok(())
@@ -244,8 +244,8 @@ impl RawClient {
     }
 
     fn callback<P: Into<Payload>>(&self, event: &Event, payload: P) -> Result<()> {
-        let mut on = self.on.write()?;
-        let mut on_any = self.on_any.write()?;
+        let mut on = self.on.lock()?;
+        let mut on_any = self.on_any.lock()?;
         let lock = on.deref_mut();
         let on_any_lock = on_any.deref_mut();
 
@@ -272,7 +272,7 @@ impl RawClient {
     fn handle_ack(&self, socket_packet: &Packet) -> Result<()> {
         let mut to_be_removed = Vec::new();
         if let Some(id) = socket_packet.id {
-            for (index, ack) in self.outstanding_acks.write()?.iter_mut().enumerate() {
+            for (index, ack) in self.outstanding_acks.lock()?.iter_mut().enumerate() {
                 if ack.id == id {
                     to_be_removed.push(index);
 
@@ -297,7 +297,7 @@ impl RawClient {
                 }
             }
             for index in to_be_removed {
-                self.outstanding_acks.write()?.remove(index);
+                self.outstanding_acks.lock()?.remove(index);
             }
         }
         Ok(())


Test patch: