Merge #1

1: Fix hanging when a future wakes itself r=bertptrs a=bertptrs

It's possible that either the future wakes itself or the something requests to be awoken while the future is being polled. This PR adds a bool to the waker state to track whether that happened, and will immediately unpark the executor if it did.

Also add bors for nicer merging.

Co-authored-by: Bert Peters <bert@bertptrs.nl>

.github/workflows/ci.yml

@@ -14,7 +14,6 @@ jobs:
     strategy:
       matrix:
         rust:
-          - "1.51" # minimum stable rust version
           - stable
           - beta
     steps:
@@ -47,6 +46,24 @@ jobs:
           command: clippy
           args: --all-targets -- -D warnings
 
+  # Reduced build for the Minimum Supported Rust Version. It does not include the lints as some of
+  # them are out-of-date and result in false positives on the codebase, e.g. mutex_atomic.
+  msrv:
+    name: MSRV
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+
+      - uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: "1.51"
+          override: true
+
+      - uses: actions-rs/cargo@v1
+        with:
+          command: test
+
   miri:
     name: Miri
     runs-on: ubuntu-latest

CHANGELOG.md

@@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Fixed potential hanging when the waker is called while polling.
+
 ## [0.1.0]
 
 - Initial release

Cargo.toml

@@ -6,6 +6,7 @@ edition = "2018"
 license = "MIT OR Apache-2.0"
 description = "It executes futures"
 repository = "https://github.com/bertptrs/beul/"
+rust-version = "1.51"
 authors = [
     "Bert Peters",
 ]

bors.toml

@@ -0,0 +1,6 @@
+status = [
+    'MSRV',
+    'tests (stable)',
+    'tests (beta)',
+    'Miri',
+]

src/lib.rs

@@ -26,15 +26,36 @@ use std::task::Wake;
 use std::task::Waker;
 
 #[derive(Default)]
-struct CondvarWake(Condvar);
+struct CondvarWake {
+    park: Condvar,
+    awoken: Mutex<bool>,
+}
+
+impl CondvarWake {
+    pub fn park(&self) {
+        let mut guard = self.awoken.lock().unwrap_or_else(PoisonError::into_inner);
+
+        // Until we are awoken, we can park on the condvar. This also handles the case where we're
+        // awoken while we're actually polling.
+        while !*guard {
+            guard = self
+                .park
+                .wait(guard)
+                .unwrap_or_else(PoisonError::into_inner);
+        }
+
+        *guard = false;
+    }
+}
 
 impl Wake for CondvarWake {
     fn wake(self: Arc<Self>) {
-        self.0.notify_one()
+        self.wake_by_ref()
     }
 
     fn wake_by_ref(self: &Arc<Self>) {
-        self.0.notify_one()
+        *self.awoken.lock().unwrap_or_else(PoisonError::into_inner) = true;
+        self.park.notify_one();
     }
 }
 
@@ -48,14 +69,10 @@ pub fn execute<T>(f: impl Future<Output = T>) -> T {
 
     let mut context = Context::from_waker(&waker);
 
-    let mutex = Mutex::new(());
-    // Cannot panic but avoids generating the unwrap code
-    let mut guard = mutex.lock().unwrap_or_else(PoisonError::into_inner);
-
     loop {
         match pinned.as_mut().poll(&mut context) {
             Poll::Ready(value) => return value,
-            Poll::Pending => guard = wake.0.wait(guard).unwrap_or_else(PoisonError::into_inner),
+            Poll::Pending => wake.park(),
         }
     }
 }

tests/futures.rs

@@ -52,3 +52,27 @@ fn test_threaded_future() {
     // Future should be polled twice, once initially and once after the wake-up
     assert_eq!(beul::execute(future), 2);
 }
+
+#[test]
+fn test_self_waking_futures() {
+    struct SelfWakingFuture(bool);
+
+    impl Future for SelfWakingFuture {
+        type Output = ();
+
+        fn poll(mut self: std::pin::Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
+            if self.0 {
+                Poll::Ready(())
+            } else {
+                // Next time we complete
+                self.0 = true;
+                // Request to be woken up
+                cx.waker().wake_by_ref();
+
+                Poll::Pending
+            }
+        }
+    }
+
+    beul::execute(SelfWakingFuture(false));
+}