From 750d9a12512727e31c9e9bc0616d271a106d7a57 Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sat, 4 Nov 2017 03:15:50 +0100 Subject: [PATCH 1/4] inprocess: Don't use `ErrorKind::BrokenPipe` on closed sender "Broken Pipe" means the receiver end is closed -- it's just wrong to signal this error when receiving from a closed sender. (Admittedly, `Connection Reset` for closed sender is rather arbitrary: a closed sender is normally signalled as EOF, not as an error, by the OS... But it's what the other back-ends use -- and it's better than "Broken Pipe" at any rate.) --- src/platform/inprocess/mod.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/platform/inprocess/mod.rs b/src/platform/inprocess/mod.rs index 8253be356..86ad20df4 100644 --- a/src/platform/inprocess/mod.rs +++ b/src/platform/inprocess/mod.rs @@ -141,7 +141,7 @@ impl OsIpcSender { -> Result<(),MpscError> { match self.sender.borrow().send(MpscChannelMessage(data.to_vec(), ports, shared_memory_regions)) { - Err(_) => Err(MpscError::ChannelClosedError), + Err(_) => Err(MpscError::BrokenPipeError), Ok(_) => Ok(()), } } @@ -376,6 +376,7 @@ impl OsIpcSharedMemory { #[derive(Debug, PartialEq)] pub enum MpscError { ChannelClosedError, + BrokenPipeError, UnknownError, } @@ -396,7 +397,10 @@ impl From for Error { fn from(mpsc_error: MpscError) -> Error { match mpsc_error { MpscError::ChannelClosedError => { - Error::new(ErrorKind::BrokenPipe, "MPSC channel closed") + Error::new(ErrorKind::ConnectionReset, "MPSC channel sender closed") + } + MpscError::BrokenPipeError => { + Error::new(ErrorKind::BrokenPipe, "MPSC channel receiver closed") } MpscError::UnknownError => Error::new(ErrorKind::Other, "Other MPSC channel error"), } From e48898419469870062d9421272dc72725e94cb43 Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sat, 4 Nov 2017 03:32:07 +0100 Subject: [PATCH 2/4] tests: Add a check for notification or closed receiver --- src/platform/test.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/platform/test.rs b/src/platform/test.rs index a73786289..56f8bf1e5 100644 --- a/src/platform/test.rs +++ b/src/platform/test.rs @@ -699,6 +699,20 @@ fn no_senders_notification() { assert!(result.unwrap_err().channel_is_closed()); } +/// Checks that a broken pipe notification is returned by `send()` +/// after the receive end was closed. +#[test] +fn no_receiver_notification() { + let (sender, receiver) = platform::channel().unwrap(); + drop(receiver); + let data: &[u8] = b"1234567"; + let result = sender.send(data, vec![], vec![]); + assert!(result.is_err()); + // We don't have an actual method for distinguishing a "broken pipe" error -- + // but at least it's not supposed to signal the same condition as closing the sender. + assert!(!result.unwrap_err().channel_is_closed()); +} + #[test] fn shared_memory() { let (tx, rx) = platform::channel().unwrap(); From 23687a75d174931dec1ff317e625c07db9861e0f Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sat, 4 Nov 2017 03:42:56 +0100 Subject: [PATCH 3/4] inprocess: `try_recv()`: Don't report `ChannelClosedError` for "no data" condition --- src/platform/inprocess/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/platform/inprocess/mod.rs b/src/platform/inprocess/mod.rs index 86ad20df4..9bf0c156d 100644 --- a/src/platform/inprocess/mod.rs +++ b/src/platform/inprocess/mod.rs @@ -100,7 +100,8 @@ impl OsIpcReceiver { Ok(MpscChannelMessage(d,c,s)) => Ok((d, c.into_iter().map(OsOpaqueIpcChannel::new).collect(), s)), - Err(_) => Err(MpscError::ChannelClosedError), + Err(mpsc::TryRecvError::Disconnected) => Err(MpscError::ChannelClosedError), + Err(_) => Err(MpscError::UnknownError), } } } From e438c8a2e546f8b940ff650b22a6bf8f20a30a4e Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sat, 4 Nov 2017 03:44:24 +0100 Subject: [PATCH 4/4] tests: Add check for correct error reporting from `try_recv()` --- src/platform/test.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/platform/test.rs b/src/platform/test.rs index 56f8bf1e5..16320965b 100644 --- a/src/platform/test.rs +++ b/src/platform/test.rs @@ -744,6 +744,23 @@ fn try_recv() { assert!(rx.try_recv().is_err()); } +/// Checks that a channel closed notification is returned by `try_recv()`. +/// +/// Also checks that the "no data" notification returned by `try_recv()` +/// when no data is pending but before the channel is closed, +/// is distinguishable from the actual "channel closed" notification. +#[test] +fn no_senders_notification_try_recv() { + let (sender, receiver) = platform::channel().unwrap(); + let result = receiver.try_recv(); + assert!(result.is_err()); + assert!(!result.unwrap_err().channel_is_closed()); + drop(sender); + let result = receiver.try_recv(); + assert!(result.is_err()); + assert!(result.unwrap_err().channel_is_closed()); +} + #[test] fn try_recv_large() { let (tx, rx) = platform::channel().unwrap();