semaphore: semaphore_units: return all units when reassigned

When semaphore_units are (move-) reassigned with other units, the held units aren't currently returned to the semaphore. Call return_all before reassigning the semaphore_units object. Add a unit test reproducer. Fixes #1465 Signed-off-by: Benny Halevy <[email protected]>
scylladb · Jul 25, 2023 · c4733e5 · c4733e5
1 parent 997f6a6
commit c4733e5
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 2 deletions.
diff --git a/include/seastar/core/semaphore.hh b/include/seastar/core/semaphore.hh
@@ -482,8 +482,11 @@ public:
     semaphore_units(semaphore_units&& o) noexcept : _sem(o._sem), _n(std::exchange(o._n, 0)) {
     }
     semaphore_units& operator=(semaphore_units&& o) noexcept {
-        _sem = o._sem;
-        _n = std::exchange(o._n, 0);
+        if (this != &o) {
+            return_all();
+            _sem = o._sem;
+            _n = std::exchange(o._n, 0);
+        }
         return *this;
     }
     semaphore_units(const semaphore_units&) = delete;

diff --git a/tests/unit/semaphore_test.cc b/tests/unit/semaphore_test.cc
@@ -438,3 +438,17 @@ SEASTAR_THREAD_TEST_CASE(test_semaphore_abort_before_wait) {
     BOOST_CHECK_THROW(fut1.get(), semaphore_aborted);
     BOOST_REQUIRE_EQUAL(x, 0);
 }
+
+SEASTAR_THREAD_TEST_CASE(test_reassigned_units_are_returned) {
+    auto sem0 = semaphore(1);
+    auto sem1 = semaphore(1);
+    auto units = get_units(sem0, 1).get();
+    auto wait = sem0.wait(1);
+    BOOST_REQUIRE(!wait.available());
+    units = get_units(sem1, 1).get();
+    timer t([] { abort(); });
+    t.arm(1s);
+    // will hang if units are not returned when reassigned
+    wait.get();
+    t.cancel();
+}