From 77469c2ed06ee51cb3a5bac9dbac4e7f9f57c2d2 Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sun, 31 Jul 2016 02:57:38 +0200 Subject: [PATCH] inprocess: Don't wrap `OsIpcReceiver` to make it `Sync` Wrap the inprocess receiver in a simple `RefCell<>` instead of `Arc>` (again). Adding the mutex here is not a good idea: it only adds overhead and potential errors, since not all of the other backends have a `Sync` receiver either; nor does the `mpsc` mechanism `ipc-channel` is modelled after. In fact it fundamentally violates the "single receiver" aspect of `mpsc`, as outlined in https://github.com/servo/ipc-channel/pull/89#issuecomment-234394535 Users can still wrap it explicitly if they need to, and know it doesn't introduce incorrect behaviour in the specific use case. This change mostly reverts the `Receiver` part of 30b20244dc83e61f9ba09b6e3db13a8ff7ac4665 ; it doesn't re-introduce the explicit `Sync` declaration for `Receiver` though, as that was/is clearly not true without the Mutex -- nor really desirable, as explained above. This change also doesn't touch the `Sender` part, which is a different story. --- src/platform/inprocess/mod.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/platform/inprocess/mod.rs b/src/platform/inprocess/mod.rs index d447dbcec..ab4532265 100644 --- a/src/platform/inprocess/mod.rs +++ b/src/platform/inprocess/mod.rs @@ -60,13 +60,13 @@ pub fn channel() -> Result<(OsIpcSender, OsIpcReceiver),MpscError> { } pub struct OsIpcReceiver { - receiver: Arc>>>, + receiver: RefCell>>, } impl PartialEq for OsIpcReceiver { fn eq(&self, other: &OsIpcReceiver) -> bool { - self.receiver.lock().unwrap().as_ref().map(|rx| rx as *const _) == - other.receiver.lock().unwrap().as_ref().map(|rx| rx as *const _) + self.receiver.borrow().as_ref().map(|rx| rx as *const _) == + other.receiver.borrow().as_ref().map(|rx| rx as *const _) } } @@ -81,17 +81,17 @@ impl fmt::Debug for OsIpcReceiver { impl OsIpcReceiver { fn new(receiver: mpsc::Receiver) -> OsIpcReceiver { OsIpcReceiver { - receiver: Arc::new(Mutex::new(Some(receiver))), + receiver: RefCell::new(Some(receiver)), } } pub fn consume(&self) -> OsIpcReceiver { - let receiver = self.receiver.lock().unwrap().take(); + let receiver = self.receiver.borrow_mut().take(); OsIpcReceiver::new(receiver.unwrap()) } pub fn recv(&self) -> Result<(Vec, Vec, Vec),MpscError> { - let r = self.receiver.lock().unwrap(); + let r = self.receiver.borrow(); match r.as_ref().unwrap().recv() { Ok(MpscChannelMessage(d,c,s)) => Ok((d, c.into_iter().map(OsOpaqueIpcChannel::new).collect(), @@ -101,7 +101,7 @@ impl OsIpcReceiver { } pub fn try_recv(&self) -> Result<(Vec, Vec, Vec),MpscError> { - let r = self.receiver.lock().unwrap(); + let r = self.receiver.borrow(); match r.as_ref().unwrap().try_recv() { Ok(MpscChannelMessage(d,c,s)) => Ok((d, c.into_iter().map(OsOpaqueIpcChannel::new).collect(), @@ -194,7 +194,7 @@ impl OsIpcReceiverSet { let mut handles: Vec> = Vec::with_capacity(self.receivers.len()); for r in &self.receivers { - let inner_r = mem::replace(&mut *r.receiver.lock().unwrap(), None); + let inner_r = mem::replace(&mut *r.receiver.borrow_mut(), None); receivers.push(inner_r); } @@ -218,7 +218,7 @@ impl OsIpcReceiverSet { // put the receivers back for (index,r) in self.receivers.iter().enumerate() { - mem::replace(&mut *r.receiver.lock().unwrap(), mem::replace(&mut receivers[index], None)); + mem::replace(&mut *r.receiver.borrow_mut(), mem::replace(&mut receivers[index], None)); } if r_id == -1 {