From 33252d3091ec6950458528d2624942ec23d5ab4e Mon Sep 17 00:00:00 2001 From: Coldwings Date: Sat, 8 Jun 2024 11:15:55 +0800 Subject: [PATCH] FIX: lockfree MPMC queue should not fail to pop/push when queue is not empty/full (#504) * FIX: lockfree MPMC queue should not fail to pop/push when queue is not empty/full Signed-off-by: Coldwings * Old CI image not able to access repo, change to preset image Signed-off-by: Coldwings --------- Signed-off-by: Coldwings --- .github/workflows/ci.linux.arm.yml | 36 ++++++++++----------- .github/workflows/ci.linux.x86.yml | 52 +++++++++++++++--------------- .github/workflows/ci.macos.arm.yml | 12 +++---- common/lockfree_queue.h | 28 +++++----------- 4 files changed, 58 insertions(+), 70 deletions(-) diff --git a/.github/workflows/ci.linux.arm.yml b/.github/workflows/ci.linux.arm.yml index c7766b4b..98275b07 100644 --- 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 index 940ac7ca..ba225852 100644 --- 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 index c8e17718..45558cab 100644 --- 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/set-timezone@v1.2 -# with: -# timezoneLinux: "Asia/Shanghai" -# timezoneMacos: "Asia/Shanghai" -# timezoneWindows: "China Standard Time" + - uses: szenius/set-timezone@v1.2 + 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 index 8bdcaac3..51ce0074 100644 --- a/common/lockfree_queue.h +++ b/common/lockfree_queue.h @@ -179,7 +179,7 @@ class LockfreeMPMCRingQueue : public LockfreeRingQueueBase { 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 { } } 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 { } } 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 void send(const T& x) { static_assert(std::is_base_of::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