Ensure the loop callback is scheduled when the ready queue is not empty

Summary: Previously we'd call ensureLoopScheduled() but it'd be a no-op since the loop was already scheduled. Delaying the call to ensureLoopScheduled() fixes the issue

Test Plan: unit tests (FiberManager.yieldTest fails without the changes to FiberManager-inl.h)

Reviewed By: andrii@fb.com

Subscribers: folly-diffs@, yfeldblum, chalfant

FB internal diff: D1993686

Signature: t1:1993686:1429070253:af933abbbbb33868a402f1d643e4e6f5fef1be83

diff --git a/folly/experimental/fibers/FiberManager-inl.h b/folly/experimental/fibers/FiberManager-inl.h
index 45ca472a01bda04cd7c01d0bb7e1e0d481c23312..354622c1247883c0e85c60522f5e2698e5bc572d 100644 (file)
--- a/folly/experimental/fibers/FiberManager-inl.h
+++ b/folly/experimental/fibers/FiberManager-inl.h
@@ -97,6 +97,9 @@ inline void FiberManager::runReadyFiber(Fiber* fiber) {
 inline bool FiberManager::loopUntilNoReady() {
   SCOPE_EXIT {
     isLoopScheduled_ = false;
+    if (!readyFibers_.empty()) {
+      ensureLoopScheduled();
+    }
     currentFiberManager_ = nullptr;
   };
 
@@ -135,10 +138,7 @@ inline bool FiberManager::loopUntilNoReady() {
     );
   }
 
-  if (!yieldedFibers_.empty()) {
-    readyFibers_.splice(readyFibers_.end(), yieldedFibers_);
-    ensureLoopScheduled();
-  }
+  readyFibers_.splice(readyFibers_.end(), yieldedFibers_);
 
   return fibersActive_ > 0;
 }

diff --git a/folly/experimental/fibers/test/FibersTest.cpp b/folly/experimental/fibers/test/FibersTest.cpp
index 681e608f96defde30dd3837bd4bf1806c7149c28..bc7a738051828dad697cccb995de87605b78703d 100644 (file)
--- a/folly/experimental/fibers/test/FibersTest.cpp
+++ b/folly/experimental/fibers/test/FibersTest.cpp
@@ -1281,6 +1281,31 @@ TEST(FiberManager, fiberLocalHeap) {
   testFiberLocal<LargeData>();
 }
 
+TEST(FiberManager, yieldTest) {
+  FiberManager manager(folly::make_unique<SimpleLoopController>());
+  auto& loopController =
+    dynamic_cast<SimpleLoopController&>(manager.loopController());
+
+  bool checkRan = false;
+
+  manager.addTask(
+    [&]() {
+      manager.yield();
+      checkRan = true;
+    }
+  );
+
+  loopController.loop(
+    [&]() {
+      if (checkRan) {
+        loopController.stop();
+      }
+    }
+  );
+
+  EXPECT_TRUE(checkRan);
+}
+
 static size_t sNumAwaits;
 
 void runBenchmark(size_t numAwaits, size_t toSend) {