FIX: lockfree MPMC queue should not fail to pop/push when queue is no…

…t empty/full (alibaba#504) * FIX: lockfree MPMC queue should not fail to pop/push when queue is not empty/full Signed-off-by: Coldwings <[email protected]> * Old CI image not able to access repo, change to preset image Signed-off-by: Coldwings <[email protected]> --------- Signed-off-by: Coldwings <[email protected]>
Coldwings · Jun 8, 2024 · 33252d3 · 33252d3
1 parent 1738a58
commit 33252d3
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 70 deletions.
diff --git a/.github/workflows/ci.linux.arm.yml b/.github/workflows/ci.linux.arm.yml
@@ -11,7 +11,7 @@ jobs:
     runs-on: [self-hosted, Linux, ARM64]
 
     container:
-      image: dokken/centos-stream-8:sha-40294ce
+      image: ghcr.io/coldwings/photon-ut-base:latest
       options: --cpus 4
 
     steps:
@@ -23,14 +23,14 @@ jobs:
 
       - uses: actions/checkout@v3
 
-      - name: Install Dependencies
-        run: |
-          dnf install -y git gcc-c++ cmake 'dnf-command(config-manager)'
-          dnf install -y gcc-toolset-9-gcc-c++
-          dnf install -y openssl-devel libcurl-devel libaio-devel
-          dnf install -y epel-release
-          dnf config-manager --set-enabled powertools
-          dnf install -y gtest-devel gmock-devel gflags-devel fuse-devel libgsasl-devel e2fsprogs-devel
+      # - name: Install Dependencies
+      #   run: |
+      #     dnf install -y git gcc-c++ cmake 'dnf-command(config-manager)'
+      #     dnf install -y gcc-toolset-9-gcc-c++
+      #     dnf install -y openssl-devel libcurl-devel libaio-devel
+      #     dnf install -y epel-release
+      #     dnf config-manager --set-enabled powertools
+      #     dnf install -y gtest-devel gmock-devel gflags-devel fuse-devel libgsasl-devel e2fsprogs-devel
 
       - name: Build
         run: |
@@ -51,7 +51,7 @@ jobs:
     runs-on: [self-hosted, Linux, ARM64]
 
     container:
-      image: dokken/centos-stream-8:sha-40294ce
+      image: ghcr.io/coldwings/photon-ut-base:latest
       options: --cpus 4
 
     steps:
@@ -63,14 +63,14 @@ jobs:
 
       - uses: actions/checkout@v3
 
-      - name: Install Dependencies
-        run: |
-          dnf install -y git gcc-c++ cmake 'dnf-command(config-manager)'
-          dnf install -y gcc-toolset-9-gcc-c++
-          dnf install -y openssl-devel libcurl-devel libaio-devel
-          dnf install -y epel-release
-          dnf config-manager --set-enabled powertools
-          dnf install -y gtest-devel gmock-devel gflags-devel fuse-devel libgsasl-devel e2fsprogs-devel
+      # - name: Install Dependencies
+      #   run: |
+      #     dnf install -y git gcc-c++ cmake 'dnf-command(config-manager)'
+      #     dnf install -y gcc-toolset-9-gcc-c++
+      #     dnf install -y openssl-devel libcurl-devel libaio-devel
+      #     dnf install -y epel-release
+      #     dnf config-manager --set-enabled powertools
+      #     dnf install -y gtest-devel gmock-devel gflags-devel fuse-devel libgsasl-devel e2fsprogs-devel
 
       - name: Build
         run: |

diff --git a/.github/workflows/ci.linux.x86.yml b/.github/workflows/ci.linux.x86.yml
@@ -11,7 +11,7 @@ jobs:
     runs-on: [self-hosted, Linux, X64]
 
     container:
-      image: dokken/centos-stream-8:sha-40294ce
+      image: ghcr.io/coldwings/photon-ut-base:latest
       options: --cpus 4
 
     steps:
@@ -23,14 +23,14 @@ jobs:
 
       - uses: actions/checkout@v3
 
-      - name: Install Dependencies
-        run: |
-          dnf install -y git gcc-c++ cmake 'dnf-command(config-manager)'
-          dnf install -y gcc-toolset-9-gcc-c++
-          dnf install -y openssl-devel libcurl-devel libaio-devel
-          dnf install -y epel-release
-          dnf config-manager --set-enabled powertools
-          dnf install -y gtest-devel gmock-devel gflags-devel fuse-devel libgsasl-devel e2fsprogs-devel
+      # - name: Install Dependencies
+      #   run: |
+      #     dnf install -y git gcc-c++ cmake 'dnf-command(config-manager)'
+      #     dnf install -y gcc-toolset-9-gcc-c++
+      #     dnf install -y openssl-devel libcurl-devel libaio-devel
+      #     dnf install -y epel-release
+      #     dnf config-manager --set-enabled powertools
+      #     dnf install -y gtest-devel gmock-devel gflags-devel fuse-devel libgsasl-devel e2fsprogs-devel
 
       - name: Build
         run: |
@@ -51,7 +51,7 @@ jobs:
     runs-on: [self-hosted, Linux, X64]
 
     container:
-      image: dokken/centos-stream-8:sha-40294ce
+      image: ghcr.io/coldwings/photon-ut-base:latest
       options: --cpus 4
 
     steps:
@@ -63,14 +63,14 @@ jobs:
 
       - uses: actions/checkout@v3
 
-      - name: Install Dependencies
-        run: |
-          dnf install -y git gcc-c++ cmake 'dnf-command(config-manager)'
-          dnf install -y gcc-toolset-9-gcc-c++
-          dnf install -y openssl-devel libcurl-devel libaio-devel
-          dnf install -y epel-release
-          dnf config-manager --set-enabled powertools
-          dnf install -y gtest-devel gmock-devel gflags-devel fuse-devel libgsasl-devel e2fsprogs-devel
+      # - name: Install Dependencies
+      #   run: |
+      #     dnf install -y git gcc-c++ cmake 'dnf-command(config-manager)'
+      #     dnf install -y gcc-toolset-9-gcc-c++
+      #     dnf install -y openssl-devel libcurl-devel libaio-devel
+      #     dnf install -y epel-release
+      #     dnf config-manager --set-enabled powertools
+      #     dnf install -y gtest-devel gmock-devel gflags-devel fuse-devel libgsasl-devel e2fsprogs-devel
 
       - name: Build
         run: |
@@ -91,7 +91,7 @@ jobs:
     runs-on: [self-hosted, Linux, X64]
 
     container:
-      image: dokken/centos-stream-8:sha-40294ce
+      image: ghcr.io/coldwings/photon-ut-base:latest
       # In order to run io_uring, the docker daemon should add --default-ulimit memlock=-1:-1
       options: --cpus 4
 
@@ -104,13 +104,13 @@ jobs:
 
       - uses: actions/checkout@v3
 
-      - name: Install Dependencies
-        run: |
-          dnf install -y git gcc-c++ cmake
-          dnf install -y gcc-toolset-9-gcc-c++
-          dnf install -y openssl-devel libcurl-devel
-          dnf install -y epel-release
-          dnf install -y fuse-devel libgsasl-devel e2fsprogs-devel
+      # - name: Install Dependencies
+      #   run: |
+      #     dnf install -y git gcc-c++ cmake
+      #     dnf install -y gcc-toolset-9-gcc-c++
+      #     dnf install -y openssl-devel libcurl-devel
+      #     dnf install -y epel-release
+      #     dnf install -y fuse-devel libgsasl-devel e2fsprogs-devel
 
       - name: Build
         run: |

diff --git a/.github/workflows/ci.macos.arm.yml b/.github/workflows/ci.macos.arm.yml
@@ -8,14 +8,14 @@ on:
 
 jobs:
   macOS-clang-debug:
-    runs-on: [self-hosted, macOS, ARM64]
+    runs-on: macos-14
 
     steps:
-#      - uses: szenius/[email protected]
-#        with:
-#          timezoneLinux: "Asia/Shanghai"
-#          timezoneMacos: "Asia/Shanghai"
-#          timezoneWindows: "China Standard Time"
+      - uses: szenius/[email protected]
+        with:
+          timezoneLinux: "Asia/Shanghai"
+          timezoneMacos: "Asia/Shanghai"
+          timezoneWindows: "China Standard Time"
 
       - uses: actions/checkout@v3
 

diff --git a/common/lockfree_queue.h b/common/lockfree_queue.h
@@ -179,7 +179,7 @@ class LockfreeMPMCRingQueue : public LockfreeRingQueueBase<T, N> {
     using Base::empty;
     using Base::full;
 
-    bool push_weak(const T& x) {
+    bool push(const T& x) {
         auto t = tail.load(std::memory_order_acquire);
         for (;;) {
             auto& slot = slots[idx(t)];
@@ -192,15 +192,16 @@ class LockfreeMPMCRingQueue : public LockfreeRingQueueBase<T, N> {
                 }
             } else {
                 auto const prevTail = t;
+                auto h = head.load(std::memory_order_acquire);
                 t = tail.load(std::memory_order_acquire);
-                if (t == prevTail) {
+                if (t == prevTail && Base::check_full(h, t)) {
                     return false;
                 }
             }
         }
     }
 
-    bool pop_weak(T& x) {
+    bool pop(T& x) {
         auto h = head.load(std::memory_order_acquire);
         for (;;) {
             auto& slot = slots[idx(h)];
@@ -213,28 +214,15 @@ class LockfreeMPMCRingQueue : public LockfreeRingQueueBase<T, N> {
                 }
             } else {
                 auto const prevHead = h;
+                auto t = tail.load(std::memory_order_acquire);
                 h = head.load(std::memory_order_acquire);
-                if (h == prevHead) {
+                if (h == prevHead && Base::check_empty(h, t)) {
                     return false;
                 }
             }
         }
     }
 
-    bool push(const T& x) {
-        do {
-            if (push_weak(x)) return true;
-        } while (!full());
-        return false;
-    }
-
-    bool pop(T& x) {
-        do {
-            if (pop_weak(x)) return true;
-        } while (!empty());
-        return false;
-    }
-
     template <typename Pause = ThreadPause>
     void send(const T& x) {
         static_assert(std::is_base_of<PauseBase, Pause>::value,
@@ -536,8 +524,8 @@ namespace common {
  * and load balancing.
  * Watch out that `recv` should run in photon environment (because it has to)
  * use photon semaphore to be notified that new item has sended. `send` could
- * running in photon or std::thread environment (needs to set template `Pause` as
- * `ThreadPause`).
+ * running in photon or std::thread environment (needs to set template `Pause`
+ * as `ThreadPause`).
  *
  * @tparam QueueType shoulde be one of LockfreeMPMCRingQueue,
  * LockfreeBatchMPMCRingQueue, or LockfreeSPSCRingQueue, with their own template