Bug 1410702 - P1: Handle errors in send_recv! r?kinetik draft
authorDan Glastonbury <dan.glastonbury@gmail.com>
Wed, 25 Oct 2017 14:53:03 +1000
changeset 685923 f4708359411a65fba620e29b235dffbe052e11e9
parent 685922 20380c6e889ceea89c8b323c629b5e9fa5201aa8
child 685924 6e70a08743324c46d6b3eebe7c3afab202c794d0
push id86034
push userbmo:dglastonbury@mozilla.com
push dateWed, 25 Oct 2017 05:03:02 +0000
reviewerskinetik
bugs1410702
milestone58.0a1
Bug 1410702 - P1: Handle errors in send_recv! r?kinetik Connection handling code wasn't handling errors from receive() properly. Attempting to unwrap on an Err was causing a panic. MozReview-Commit-ID: GURe3FHPbjT
media/audioipc/audioipc/src/connection.rs
media/audioipc/audioipc/src/messages.rs
media/audioipc/client/src/send_recv.rs
media/audioipc/server/src/lib.rs
--- a/media/audioipc/audioipc/src/connection.rs
+++ b/media/audioipc/audioipc/src/connection.rs
@@ -4,16 +4,17 @@ use bytes::{BufMut, BytesMut};
 use codec::{Decoder, encode};
 use errors::*;
 use mio::{Poll, PollOpt, Ready, Token};
 use mio::event::Evented;
 use mio::unix::EventedFd;
 use serde::de::DeserializeOwned;
 use serde::ser::Serialize;
 use std::collections::VecDeque;
+use std::error::Error;
 use std::fmt::Debug;
 use std::io::{self, Read};
 use std::os::unix::io::{AsRawFd, RawFd};
 use std::os::unix::net;
 use std::os::unix::prelude::*;
 
 // Because of the trait implementation rules in Rust, this needs to be
 // a wrapper class to allow implementation of a trait from another
@@ -121,17 +122,17 @@ impl Connection {
                         self.recv_fd.push_back(
                             unsafe { AutoCloseFd::from_raw_fd(fd) }
                         );
                     }
                 },
                 Ok(Async::NotReady) => bail!("Socket should be blocking."),
                 // TODO: Handle dropped message.
                 // Err(ref e) if e.kind() == std::io::ErrorKind::WouldBlock => panic!("wouldblock"),
-                _ => bail!("socket write"),
+                Err(e) => bail!("stream recv_buf_fd returned: {}", e.description()),
             }
         }
     }
 
     pub fn send<ST>(&mut self, msg: ST) -> Result<usize>
     where
         ST: Serialize + Debug,
     {
--- a/media/audioipc/audioipc/src/messages.rs
+++ b/media/audioipc/audioipc/src/messages.rs
@@ -238,12 +238,10 @@ pub enum ClientMessage {
     StreamLatency(u32),
     StreamVolumeSet,
     StreamPanningSet,
     StreamCurrentDevice(Device),
 
     StreamDataCallback(isize, usize),
     StreamStateCallback(ffi::cubeb_state),
 
-    ContextError(ffi::cubeb_error_code),
-    StreamError, /*(Error)*/
-    ClientError /*(Error)*/
+    Error(ffi::cubeb_error_code)
 }
--- a/media/audioipc/client/src/send_recv.rs
+++ b/media/audioipc/client/src/send_recv.rs
@@ -27,26 +27,40 @@ macro_rules! send_recv {
     (__send $conn:expr, $smsg:ident, $($a:expr),*) => ({
         let r = $conn.send(ServerMessage::$smsg($($a),*));
         if r.is_err() {
             debug!("send error - got={:?}", r);
             return Err(ErrorCode::Error.into());
         }
     });
     (__recv $conn:expr, $rmsg:ident) => ({
-        let r = $conn.receive().unwrap();
-        if let ClientMessage::$rmsg = r {
-            Ok(())
-        } else {
-            debug!("receive error - got={:?}", r);
-            Err(ErrorCode::Error.into())
+        match $conn.receive() {
+            Ok(ClientMessage::$rmsg) => Ok(()),
+            // Macro can see ErrorCode but not Error? I don't understand.
+            // ::cubeb_core::Error is fragile but this isn't general purpose code.
+            Ok(ClientMessage::Error(e)) => Err(unsafe { ::cubeb_core::Error::from_raw(e) }),
+            Ok(m) => {
+                debug!("receive unexpected message - got={:?}", m);
+                Err(ErrorCode::Error.into())
+            },
+            Err(e) => {
+                debug!("receive error - got={:?}", e);
+                Err(ErrorCode::Error.into())
+            },
         }
     });
     (__recv $conn:expr, $rmsg:ident __result) => ({
-        let r = $conn.receive().unwrap();
-        if let ClientMessage::$rmsg(v) = r {
-            Ok(v)
-        } else {
-            debug!("receive error - got={:?}", r);
-            Err(ErrorCode::Error.into())
+        match $conn.receive() {
+            Ok(ClientMessage::$rmsg(v)) => Ok(v),
+            // Macro can see ErrorCode but not Error? I don't understand.
+            // ::cubeb_core::Error is fragile but this isn't general purpose code.
+            Ok(ClientMessage::Error(e)) => Err(unsafe { ::cubeb_core::Error::from_raw(e) }),
+            Ok(m) => {
+                debug!("receive unexpected message - got={:?}", m);
+                Err(ErrorCode::Error.into())
+            },
+            Err(e) => {
+                debug!("receive error - got={:?}", e);
+                Err(ErrorCode::Error.into())
+            },
         }
     })
 }
--- a/media/audioipc/server/src/lib.rs
+++ b/media/audioipc/server/src/lib.rs
@@ -758,17 +758,17 @@ pub fn run(running: Arc<AtomicBool>) -> 
             return r;
         }
     }
 
     //poll.deregister(&server.socket).unwrap();
 }
 
 fn error(error: cubeb::Error) -> ClientMessage {
-    ClientMessage::ContextError(error.raw_code())
+    ClientMessage::Error(error.raw_code())
 }
 
 struct ServerWrapper {
     thread_handle: std::thread::JoinHandle<()>,
     sender_ctl: channel::SenderCtl,
     path: std::path::PathBuf,
 }