Don't make OsIpcSender Sync

.travis.yml

@@ -8,12 +8,12 @@ os:
   - osx
 
 env:
-  - FEATURES=""
-  - FEATURES="--features force-inprocess"
+  - FEATURES="unstable"
+  - FEATURES="unstable force-inprocess"
 
 notifications:
   webhooks: http://build.servo.org:54856/travis
 
 script:
-  - cargo build $FEATURES
-  - cargo test $FEATURES
+  - cargo build --features "$FEATURES"
+  - cargo test --features "$FEATURES"

Cargo.toml

@@ -1,13 +1,14 @@
 [package]
 name = "ipc-channel"
-version = "0.5.1"
+version = "0.6.0"
 description = "A multiprocess drop-in replacement for Rust channels"
 authors = ["The Servo Project Developers"]
 license = "MIT/Apache-2.0"
 repository = "https://github.com/servo/ipc-channel"
 
 [features]
 force-inprocess = []
+unstable = []
 
 [dependencies]
 bincode = "0.6"

appveyor.yml

@@ -13,4 +13,4 @@ install:
 build: false
 
 test_script:
-  - cargo test --verbose
+  - 'cargo test --verbose --features "unstable"'

src/lib.rs

@@ -9,6 +9,7 @@
 
 #![cfg_attr(any(feature = "force-inprocess", target_os = "windows", target_os = "android"),
 			feature(mpsc_select))]
+#![cfg_attr(all(feature = "unstable", test), feature(specialization))]
 
 #[macro_use]
 extern crate lazy_static;

src/platform/inprocess/mod.rs

@@ -106,20 +106,20 @@ impl OsIpcReceiver {
 
 #[derive(Clone, Debug)]
 pub struct OsIpcSender {
-    sender: Arc<Mutex<mpsc::Sender<MpscChannelMessage>>>,
+    sender: RefCell<mpsc::Sender<MpscChannelMessage>>,
 }
 
 impl PartialEq for OsIpcSender {
     fn eq(&self, other: &OsIpcSender) -> bool {
-        &*self.sender.lock().unwrap() as *const _ ==
-            &*other.sender.lock().unwrap() as *const _
+        &*self.sender.borrow() as *const _ ==
+            &*other.sender.borrow() as *const _
     }
 }
 
 impl OsIpcSender {
     fn new(sender: mpsc::Sender<MpscChannelMessage>) -> OsIpcSender {
         OsIpcSender {
-            sender: Arc::new(Mutex::new(sender)),
+            sender: RefCell::new(sender),
         }
     }
 
@@ -139,7 +139,7 @@ impl OsIpcSender {
                 shared_memory_regions: Vec<OsIpcSharedMemory>)
                 -> Result<(),MpscError>
     {
-        match self.sender.lock().unwrap().send(MpscChannelMessage(data.to_vec(), ports, shared_memory_regions)) {
+        match self.sender.borrow().send(MpscChannelMessage(data.to_vec(), ports, shared_memory_regions)) {
             Err(_) => Err(MpscError::ChannelClosedError),
             Ok(_) => Ok(()),
         }

src/platform/macos/mod.rs

@@ -19,6 +19,7 @@ use std::cell::Cell;
 use std::ffi::CString;
 use std::fmt::{self, Debug, Formatter};
 use std::io::{Error, ErrorKind};
+use std::marker::PhantomData;
 use std::mem;
 use std::ops::Deref;
 use std::ptr;
@@ -306,6 +307,11 @@ impl OsIpcReceiver {
 #[derive(PartialEq, Debug)]
 pub struct OsIpcSender {
     port: mach_port_t,
+    // Make sure this is `!Sync`, to match `mpsc::Sender`; and to discourage sharing references.
+    //
+    // (Rather, senders should just be cloned, as they are shared internally anyway --
+    // another layer of sharing only adds unnecessary overhead...)
+    nosync_marker: PhantomData<Cell<()>>,
 }
 
 impl Drop for OsIpcSender {
@@ -334,6 +340,7 @@ impl Clone for OsIpcSender {
         }
         OsIpcSender {
             port: self.port,
+            nosync_marker: PhantomData,
         }
     }
 }
@@ -342,6 +349,7 @@ impl OsIpcSender {
     fn from_name(port: mach_port_t) -> OsIpcSender {
         OsIpcSender {
             port: port,
+            nosync_marker: PhantomData,
         }
     }
 
@@ -480,6 +488,7 @@ impl OsOpaqueIpcChannel {
     pub fn to_sender(&mut self) -> OsIpcSender {
         OsIpcSender {
             port: mem::replace(&mut self.port, MACH_PORT_NULL),
+            nosync_marker: PhantomData,
         }
     }
 

src/platform/test.rs

@@ -629,3 +629,27 @@ fn try_recv_large_delayed() {
         thread.join().unwrap();
     }
 }
+
+#[cfg(feature = "unstable")]
+mod sync_test {
+    use platform;
+
+    trait SyncTest {
+        fn test_not_sync();
+    }
+
+    impl<T> SyncTest for T {
+        default fn test_not_sync() {}
+    }
+
+    impl<T: Sync> SyncTest for T {
+        fn test_not_sync() {
+            panic!("`OsIpcSender` should not be `Sync`");
+        }
+    }
+
+    #[test]
+    fn receiver_not_sync() {
+        platform::OsIpcSender::test_not_sync();
+    }
+}

src/platform/unix/mod.rs

@@ -12,11 +12,13 @@ use libc::{self, MAP_FAILED, MAP_SHARED, POLLIN, PROT_READ, PROT_WRITE, SOCK_SEQ
 use libc::{SO_LINGER, S_IFMT, S_IFSOCK, c_char, c_int, c_void, getsockopt};
 use libc::{iovec, mkstemp, mode_t, msghdr, nfds_t, off_t, poll, pollfd, recvmsg, sendmsg};
 use libc::{setsockopt, size_t, sockaddr, sockaddr_un, socketpair, socklen_t, sa_family_t};
+use std::cell::Cell;
 use std::cmp;
 use std::collections::HashSet;
 use std::ffi::{CStr, CString};
 use std::fmt::{self, Debug, Formatter};
 use std::io::Error;
+use std::marker::PhantomData;
 use std::mem;
 use std::ops::Deref;
 use std::ptr;
@@ -124,6 +126,11 @@ pub struct SharedFileDescriptor(c_int);
 #[derive(PartialEq, Debug, Clone)]
 pub struct OsIpcSender {
     fd: Arc<SharedFileDescriptor>,
+    // Make sure this is `!Sync`, to match `mpsc::Sender`; and to discourage sharing references.
+    //
+    // (Rather, senders should just be cloned, as they are shared internally anyway --
+    // another layer of sharing only adds unnecessary overhead...)
+    nosync_marker: PhantomData<Cell<()>>,
 }
 
 impl Drop for SharedFileDescriptor {
@@ -139,6 +146,7 @@ impl OsIpcSender {
     fn from_fd(fd: c_int) -> OsIpcSender {
         OsIpcSender {
             fd: Arc::new(SharedFileDescriptor(fd)),
+            nosync_marker: PhantomData,
         }
     }