Panic inside a future is propagated into its coroutine

If a future panics while being polled, it doesn't kill the core, but is properly removed from there and the panic resumes inside the coroutine responsible for the future. This both protects the core and makes the behavior more natural.
vorner · Jan 18, 2018 · 0f8722fdbf0efa1f2c21a4f18791e87cb56e41d4 · 0f8722f
1 parent a3abbb6
commit 0f8722fdbf0efa1f2c21a4f18791e87cb56e41d4
Unified Split

Showing with 74 additions and 22 deletions.

+2 −0 CHANGELOG.md

+0 −2 TODO

+34 −3 src/coroutine.rs

+14 −10 src/prelude.rs

+24 −7 src/switch.rs
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,5 @@
+  
+* A panic inside a future propagates to the owning coroutine, doesn't kill the
+  
+  whole core.
 * The `spawn` method no longer catches panic by default. The
  `spawn_catch_panic`.

diff --git a/TODO b/TODO
@@ -7,8 +7,6 @@
 # Panicking/unwinding
-  
-* Make sure panic inside of a future is propagated into the coroutine that
-  
-  called it.
 * Coroutine::scoped & Coroutine::main
 # Interface

diff --git a/src/coroutine.rs b/src/coroutine.rs
@@ -318,6 +318,8 @@ impl Coroutine {
    /// # Panics
    ///
    /// If called outside of a coroutine (there's nothing to suspend).
+  
+    ///
+  
+    /// Also, panics from withit the provided future are propagated into the calling coroutine.
    pub fn wait<I, E, Fut>(mut fut: Fut) -> Result<Result<I, E>, Dropped>
    where
        Fut: Future<Item = I, Error = E>,
@@ -387,8 +389,9 @@ impl Coroutine {
            instruction.exchange(my_context.parent_context)
        };
        let (result, stack) = match reply_instruction {
-  
-            Switch::Resume { stack } => (Ok(result.unwrap()), stack),
-  
-            Switch::Cleanup { stack } => (Err(Dropped), stack),
+  
+            Switch::Resume { stack } => (Ok(Ok(result.unwrap())), stack),
+  
+            Switch::Cleanup { stack } => (Ok(Err(Dropped)), stack),
+  
+            Switch::PropagateFuturePanic { stack, panic } => (Err(panic), stack),
            _ => unreachable!("Invalid instruction on wakeup"),
        };
        // Reconstruct our context anew after we switched back.
@@ -399,7 +402,10 @@ impl Coroutine {
            leak_on_panic: my_context.leak_on_panic,
        };
        CONTEXTS.with(|c| c.borrow_mut().push(new_context));
-  
-        result
+  
+        match result {
+  
+            Ok(result) => result,
+  
+            Err(panic) => panic::resume_unwind(panic),
+  
+        }
    }
 }
@@ -531,4 +537,29 @@ mod tests {
        }
        */
    }
+  
+
+  
+    /// A panic from inside the future doesn't kill the core, but falls out of the wait into the
+  
+    /// responsible coroutine.
+  
+    ///
+  
+    /// We test this separately because the future is „exported“ to the main coroutine to be
+  
+    /// polled. So we check it doesn't kill the core.
+  
+    #[test]
+  
+    fn panic_in_future() {
+  
+        let mut core = Core::new().unwrap();
+  
+        let coroutine = Coroutine::with_defaults(core.handle(), || {
+  
+            struct PanicFuture;
+  
+            impl Future for PanicFuture {
+  
+                type Item = ();
+  
+                type Error = ();
+  
+                fn poll(&mut self) -> Poll<(), ()> {
+  
+                    panic!("Test");
+  
+                }
+  
+            }
+  
+
+  
+            if let Ok(_) = panic::catch_unwind(|| Coroutine::wait(PanicFuture)) {
+  
+                panic!("A panic should fall out of wait");
+  
+            }
+  
+        });
+  
+        core.run(coroutine).unwrap();
+  
+    }
 }
diff --git a/src/prelude.rs b/src/prelude.rs
@@ -44,7 +44,8 @@ pub trait CoroutineFuture: Sized {
    /// This'll panic if the reactor the coroutine was spawned onto is dropped while the method
    /// runs.
    ///
-  
-    /// It also panics when called outside of the coroutine.
+  
+    /// It also panics when called outside of the coroutine and any panics from the coroutine
+  
+    /// itself will be propagated to the calling coroutine.
    ///
    /// # Examples
    ///
@@ -78,7 +79,8 @@ pub trait CoroutineFuture: Sized {
    ///
    /// # Panics
    ///
-  
-    /// When called outside of the coroutine.
+  
+    /// When called outside of the coroutine. Also, panics from within the future are propagated to
+  
+    /// the calling (current) coroutine.
    fn coro_wait_cleanup(self) -> Result<Result<Self::Item, Self::Error>, Dropped>;
 }
@@ -117,7 +119,8 @@ pub trait CoroutineStream: Sized {
    /// If the reactor is dropped during the iteration, this method panics to clean up the
    /// coroutine.
    ///
-  
-    /// It also panics when called from outside of a coroutine.
+  
+    /// It also panics when called from outside of a coroutine. Panics from within the stream are
+  
+    /// propagated into the calling coroutine.
    ///
    /// # Examples
    ///
@@ -170,7 +173,8 @@ pub trait CoroutineStream: Sized {
    ///
    /// This panics when the reactor the current coroutine runs on is dropped while iterating.
    ///
-  
-    /// It panics when called outside of a coroutine.
+  
+    /// It panics when called outside of a coroutine and any panics from within the stream are
+  
+    /// propagated to the calling coroutine.
    ///
    /// # Examples
    ///
@@ -212,7 +216,7 @@ pub trait CoroutineStream: Sized {
    ///
    /// # Panics
    ///
-  
-    /// If called outside of a coroutine.
+  
+    /// If called outside of a coroutine, or if the stream itself panics internally.
    fn iter_cleanup(self) -> CleanupIterator<Self>;
    /// A future that pulls one item out of the stream.
@@ -261,7 +265,7 @@ pub trait CoroutineStream: Sized {
    ///
    /// It panics when the reactor is dropped while waiting for the item.
    ///
-  
-    /// It also panics when called outside of a coroutine.
+  
+    /// It also panics when called outside of a coroutine or when the stream itself panics.
    ///
    /// # Examples
    ///
@@ -300,7 +304,7 @@ pub trait CoroutineStream: Sized {
    ///
    /// # Panics
    ///
-  
-    /// When called outside of a coroutine.
+  
+    /// When called outside of a coroutine or when the stream itself panics.
    fn coro_next_cleanup(&mut self) -> Result<Result<Option<Self::Item>, Self::Error>, Dropped>;
 }
@@ -343,7 +347,7 @@ pub trait CoroutineSink: Sized {
    ///
    /// If the reactor is dropped before the sending is done.
    ///
-  
-    /// If it is called outside of a coroutine.
+  
+    /// If it is called outside of a coroutine or if the sink panics internally.
    ///
    /// # Examples
    ///
@@ -378,7 +382,7 @@ pub trait CoroutineSink: Sized {
    ///
    /// # Panics
    ///
-  
-    /// If it is called outside of a coroutine.
+  
+    /// If it is called outside of a coroutine or if the sink itself panics.
    fn coro_send_cleanup(&mut self, item: Self::Item) -> Result<Result<(), Self::Error>, Dropped>;
    /// Sends multiple items into the sink.
@@ -393,7 +397,7 @@ pub trait CoroutineSink: Sized {
    ///
    /// # Panics
    ///
-  
-    /// If it is called outside of a coroutine.
+  
+    /// If it is called outside of a coroutine or if the sink panics internally.
    fn coro_send_many<Iter, I>(&mut self, iter: I) -> Result<Result<(), Self::Error>, Dropped>
    where
        Iter: Iterator<Item = Self::Item>,

diff --git a/src/switch.rs b/src/switch.rs
@@ -1,7 +1,7 @@
 //! Module for the low-level switching of coroutines
 use std::any::Any;
-  
-use std::panic;
+  
+use std::panic::{self, AssertUnwindSafe};
 use std::thread;
 use context::{Context, Transfer};
@@ -46,16 +46,27 @@ impl Future for WaitTask {
    type Error = ();
    fn poll(&mut self) -> Poll<(), ()> {
        assert!(self.context.is_some());
-  
-        match (self.poll.as_mut().unwrap())() {
-  
-            Ok(Async::NotReady) => Ok(Async::NotReady),
-  
-            other => {
+  
+        // The catch unwind is fine ‒ we don't swallow the panic, only move it to the correct place
+  
+        // ‒ so likely everything relevant will be dropped like with any other normal panic.
+  
+        match panic::catch_unwind(AssertUnwindSafe(self.poll.as_mut().unwrap())) {
+  
+            Ok(Ok(Async::NotReady)) => Ok(Async::NotReady),
+  
+            Ok(result) => {
                drop(self.poll.take());
                Switch::Resume {
-  
-                        stack: self.stack.take().unwrap()
+  
+                        stack: self.stack.take().unwrap(),
                    }
                    .run_child(self.context.take().unwrap());
-  
-                other
-  
-            }
+  
+                result
+  
+            },
+  
+            Err(panic) => {
+  
+                drop(self.poll.take());
+  
+                Switch::PropagateFuturePanic {
+  
+                        stack: self.stack.take().unwrap(),
+  
+                        panic
+  
+                    }
+  
+                    .run_child(self.context.take().unwrap());
+  
+                Err(())
+  
+            },
        }
    }
 }
@@ -123,9 +134,15 @@ pub(crate) enum Switch {
        stack: ProtectedFixedSizeStack,
        task: BoxedTask,
    },
+  
+    /// Wait on a future to finish
    WaitFuture {
        task: WaitTask,
    },
+  
+    /// A future panicked, propagate it into the coroutine.
+  
+    PropagateFuturePanic {
+  
+        stack: ProtectedFixedSizeStack,
+  
+        panic: Box<Any + Send>,
+  
+    },
    /// Continue operation, the future is resolved.
    Resume {
        stack: ProtectedFixedSizeStack,