Skip to content

Fix ABA and UAF problem in ConditionVariable notify operation using internal spinlock (#458)#459

Merged
ChuanqiXu9 merged 4 commits intoalibaba:mainfrom
fanfanffy163:fix-cv-aba-bug
Mar 17, 2026
Merged

Fix ABA and UAF problem in ConditionVariable notify operation using internal spinlock (#458)#459
ChuanqiXu9 merged 4 commits intoalibaba:mainfrom
fanfanffy163:fix-cv-aba-bug

Conversation

@fanfanffy163
Copy link
Copy Markdown
Contributor

Fixes(#458)

Why

修复并发操作ConditionVariable的notify可能会导致UAF和ABA的线程安全问题

What is changing

给ConditionVariable的notify操作加了自旋锁,专门使notify操作串行化

…baba#458)

Serialize notifyOne and notifyAll with an internal SpinLock (MPSC model) to prevent Use-After-Free and ABA memory corruption during concurrent pops, while preserving the lock-free performance of wait().
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 16, 2026

CLA assistant check
All committers have signed the CLA.

@fanfanffy163 fanfanffy163 changed the title Fix ABA problem in ConditionVariable::notifyOne using internal spinlock (#458) Fix ABA and UAF problem in ConditionVariable notify operation using internal spinlock (#458) Mar 16, 2026
Copy link
Copy Markdown
Collaborator

@ChuanqiXu9 ChuanqiXu9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks

@fanfanffy163
Copy link
Copy Markdown
Contributor Author

应该是cppm的依赖顺序问题,我去调一下

@fanfanffy163
Copy link
Copy Markdown
Contributor Author

@poor-circle @ChuanqiXu9 非常感谢两位大佬的指导,ci问题已修复,麻烦大佬们再帮忙Review一下。

@fanfanffy163
Copy link
Copy Markdown
Contributor Author

The diff you sent is not formatted correctly.
The suggested format is
diff --git a/async_simple/async_simple.cppm b/async_simple/async_simple.cppm
index 35fd598..4957222 100644
--- a/async_simple/async_simple.cppm
+++ b/async_simple/async_simple.cppm
@@ -51,35 +51,35 @@ export extern "C++" {
#if defined(clang_major) && clang_major >= 17
#include "coro/PromiseAllocator.h"
#endif

  • #include "coro/Lazy.h"
  • #include "uthread/internal/thread_impl.h"
  • #include "uthread/Await.h"
  • #include "uthread/Latch.h"
  • #include "uthread/internal/thread.h"
  • #include "uthread/Uthread.h"
  • #include "uthread/Async.h"
  • #include "coro/SpinLock.h"
  • #include "coro/ConditionVariable.h"
  • #include "coro/Latch.h"
  • #include "coro/CountEvent.h"
  • #include "coro/Collect.h"
  • #include "IOExecutor.h"
  • #include "coro/SharedMutex.h"
  • #include "uthread/Collect.h"
  • #include "executors/SimpleIOExecutor.h"
  • #include "coro/Mutex.h"
  • #include "Collect.h"
  • #include "util/Condition.h"
  • #include "coro/Dispatch.h"
  • #include "coro/Sleep.h"
  • #include "util/Queue.h"
  • #include "util/ThreadPool.h"
  • #include "coro/ResumeBySchedule.h"
  • #include "coro/FutureAwaiter.h"
  • #include "coro/SyncAwait.h"
  • #include "executors/SimpleExecutor.h"
  • #include "coro/Semaphore.h"
  • // There are some bugs in clang lower versions.
    +#include "Collect.h"
    +#include "IOExecutor.h"
    +#include "coro/Collect.h"
    +#include "coro/ConditionVariable.h"
    +#include "coro/CountEvent.h"
    +#include "coro/Dispatch.h"
    +#include "coro/FutureAwaiter.h"
    +#include "coro/Latch.h"
    +#include "coro/Lazy.h"
    +#include "coro/Mutex.h"
    +#include "coro/ResumeBySchedule.h"
    +#include "coro/Semaphore.h"
    +#include "coro/SharedMutex.h"
    +#include "coro/Sleep.h"
    +#include "coro/SpinLock.h"
    +#include "coro/SyncAwait.h"
    +#include "executors/SimpleExecutor.h"
    +#include "executors/SimpleIOExecutor.h"
    +#include "uthread/Async.h"
    +#include "uthread/Await.h"
    +#include "uthread/Collect.h"
    +#include "uthread/Latch.h"
    +#include "uthread/Uthread.h"
    +#include "uthread/internal/thread.h"
    +#include "uthread/internal/thread_impl.h"
    +#include "util/Condition.h"
    +#include "util/Queue.h"
    +#include "util/ThreadPool.h"
  • // There are some bugs in clang lower versions.
    #if defined(clang_major) && clang_major >= 17
    #include "coro/Generator.h"
    #endif
    Error: Process completed with exit code 1.

这个格式错误如果要更改的话,我怕影响到本来的先后依赖关系,Clang Format想全部按照字母顺序排。
@ChuanqiXu9 麻烦大佬帮忙看下

@ChuanqiXu9
Copy link
Copy Markdown
Collaborator

ChuanqiXu9 commented Mar 16, 2026

The diff you sent is not formatted correctly. The suggested format is diff --git a/async_simple/async_simple.cppm b/async_simple/async_simple.cppm index 35fd598..4957222 100644 --- a/async_simple/async_simple.cppm +++ b/async_simple/async_simple.cppm @@ -51,35 +51,35 @@ export extern "C++" { #if defined(clang_major) && clang_major >= 17 #include "coro/PromiseAllocator.h" #endif

* #include "coro/Lazy.h"

* #include "uthread/internal/thread_impl.h"

* #include "uthread/Await.h"

* #include "uthread/Latch.h"

* #include "uthread/internal/thread.h"

* #include "uthread/Uthread.h"

* #include "uthread/Async.h"

* #include "coro/SpinLock.h"

* #include "coro/ConditionVariable.h"

* #include "coro/Latch.h"

* #include "coro/CountEvent.h"

* #include "coro/Collect.h"

* #include "IOExecutor.h"

* #include "coro/SharedMutex.h"

* #include "uthread/Collect.h"

* #include "executors/SimpleIOExecutor.h"

* #include "coro/Mutex.h"

* #include "Collect.h"

* #include "util/Condition.h"

* #include "coro/Dispatch.h"

* #include "coro/Sleep.h"

* #include "util/Queue.h"

* #include "util/ThreadPool.h"

* #include "coro/ResumeBySchedule.h"

* #include "coro/FutureAwaiter.h"

* #include "coro/SyncAwait.h"

* #include "executors/SimpleExecutor.h"

* #include "coro/Semaphore.h"

* // There are some bugs in clang lower versions.
  +#include "Collect.h"
  +#include "IOExecutor.h"
  +#include "coro/Collect.h"
  +#include "coro/ConditionVariable.h"
  +#include "coro/CountEvent.h"
  +#include "coro/Dispatch.h"
  +#include "coro/FutureAwaiter.h"
  +#include "coro/Latch.h"
  +#include "coro/Lazy.h"
  +#include "coro/Mutex.h"
  +#include "coro/ResumeBySchedule.h"
  +#include "coro/Semaphore.h"
  +#include "coro/SharedMutex.h"
  +#include "coro/Sleep.h"
  +#include "coro/SpinLock.h"
  +#include "coro/SyncAwait.h"
  +#include "executors/SimpleExecutor.h"
  +#include "executors/SimpleIOExecutor.h"
  +#include "uthread/Async.h"
  +#include "uthread/Await.h"
  +#include "uthread/Collect.h"
  +#include "uthread/Latch.h"
  +#include "uthread/Uthread.h"
  +#include "uthread/internal/thread.h"
  +#include "uthread/internal/thread_impl.h"
  +#include "util/Condition.h"
  +#include "util/Queue.h"
  +#include "util/ThreadPool.h"


* // There are some bugs in clang lower versions.
  #if defined(**clang_major**)  && **clang_major** >= 17
  #include "coro/Generator.h"
  #endif
  Error: Process completed with exit code 1.

这个格式错误如果要更改的话,我怕影响到本来的先后依赖关系,Clang Format想全部按照字母顺序排。 @ChuanqiXu9 麻烦大佬帮忙看下

没事,format 不是强制检查。你签一下 CLA 吧,不然没法合

@fanfanffy163
Copy link
Copy Markdown
Contributor Author

The diff you sent is not formatted correctly. The suggested format is diff --git a/async_simple/async_simple.cppm b/async_simple/async_simple.cppm index 35fd598..4957222 100644 --- a/async_simple/async_simple.cppm +++ b/async_simple/async_simple.cppm @@ -51,35 +51,35 @@ export extern "C++" { #if defined(clang_major) && clang_major >= 17 #include "coro/PromiseAllocator.h" #endif

* #include "coro/Lazy.h"

* #include "uthread/internal/thread_impl.h"

* #include "uthread/Await.h"

* #include "uthread/Latch.h"

* #include "uthread/internal/thread.h"

* #include "uthread/Uthread.h"

* #include "uthread/Async.h"

* #include "coro/SpinLock.h"

* #include "coro/ConditionVariable.h"

* #include "coro/Latch.h"

* #include "coro/CountEvent.h"

* #include "coro/Collect.h"

* #include "IOExecutor.h"

* #include "coro/SharedMutex.h"

* #include "uthread/Collect.h"

* #include "executors/SimpleIOExecutor.h"

* #include "coro/Mutex.h"

* #include "Collect.h"

* #include "util/Condition.h"

* #include "coro/Dispatch.h"

* #include "coro/Sleep.h"

* #include "util/Queue.h"

* #include "util/ThreadPool.h"

* #include "coro/ResumeBySchedule.h"

* #include "coro/FutureAwaiter.h"

* #include "coro/SyncAwait.h"

* #include "executors/SimpleExecutor.h"

* #include "coro/Semaphore.h"

* // There are some bugs in clang lower versions.
  +#include "Collect.h"
  +#include "IOExecutor.h"
  +#include "coro/Collect.h"
  +#include "coro/ConditionVariable.h"
  +#include "coro/CountEvent.h"
  +#include "coro/Dispatch.h"
  +#include "coro/FutureAwaiter.h"
  +#include "coro/Latch.h"
  +#include "coro/Lazy.h"
  +#include "coro/Mutex.h"
  +#include "coro/ResumeBySchedule.h"
  +#include "coro/Semaphore.h"
  +#include "coro/SharedMutex.h"
  +#include "coro/Sleep.h"
  +#include "coro/SpinLock.h"
  +#include "coro/SyncAwait.h"
  +#include "executors/SimpleExecutor.h"
  +#include "executors/SimpleIOExecutor.h"
  +#include "uthread/Async.h"
  +#include "uthread/Await.h"
  +#include "uthread/Collect.h"
  +#include "uthread/Latch.h"
  +#include "uthread/Uthread.h"
  +#include "uthread/internal/thread.h"
  +#include "uthread/internal/thread_impl.h"
  +#include "util/Condition.h"
  +#include "util/Queue.h"
  +#include "util/ThreadPool.h"


* // There are some bugs in clang lower versions.
  #if defined(**clang_major**)  && **clang_major** >= 17
  #include "coro/Generator.h"
  #endif
  Error: Process completed with exit code 1.

这个格式错误如果要更改的话,我怕影响到本来的先后依赖关系,Clang Format想全部按照字母顺序排。 @ChuanqiXu9 麻烦大佬帮忙看下

没事,format 不是强制检查。你签一下 CLA 吧,不然没法合

好的,已同意。谢谢Review@ChuanqiXu9

@c8ef
Copy link
Copy Markdown
Collaborator

c8ef commented Mar 16, 2026

The diff you sent is not formatted correctly. The suggested format is diff --git a/async_simple/async_simple.cppm b/async_simple/async_simple.cppm index 35fd598..4957222 100644 --- a/async_simple/async_simple.cppm +++ b/async_simple/async_simple.cppm @@ -51,35 +51,35 @@ export extern "C++" { #if defined(clang_major) && clang_major >= 17 #include "coro/PromiseAllocator.h" #endif

  • #include "coro/Lazy.h"

  • #include "uthread/internal/thread_impl.h"

  • #include "uthread/Await.h"

  • #include "uthread/Latch.h"

  • #include "uthread/internal/thread.h"

  • #include "uthread/Uthread.h"

  • #include "uthread/Async.h"

  • #include "coro/SpinLock.h"

  • #include "coro/ConditionVariable.h"

  • #include "coro/Latch.h"

  • #include "coro/CountEvent.h"

  • #include "coro/Collect.h"

  • #include "IOExecutor.h"

  • #include "coro/SharedMutex.h"

  • #include "uthread/Collect.h"

  • #include "executors/SimpleIOExecutor.h"

  • #include "coro/Mutex.h"

  • #include "Collect.h"

  • #include "util/Condition.h"

  • #include "coro/Dispatch.h"

  • #include "coro/Sleep.h"

  • #include "util/Queue.h"

  • #include "util/ThreadPool.h"

  • #include "coro/ResumeBySchedule.h"

  • #include "coro/FutureAwaiter.h"

  • #include "coro/SyncAwait.h"

  • #include "executors/SimpleExecutor.h"

  • #include "coro/Semaphore.h"

  • // There are some bugs in clang lower versions.
    +#include "Collect.h"
    +#include "IOExecutor.h"
    +#include "coro/Collect.h"
    +#include "coro/ConditionVariable.h"
    +#include "coro/CountEvent.h"
    +#include "coro/Dispatch.h"
    +#include "coro/FutureAwaiter.h"
    +#include "coro/Latch.h"
    +#include "coro/Lazy.h"
    +#include "coro/Mutex.h"
    +#include "coro/ResumeBySchedule.h"
    +#include "coro/Semaphore.h"
    +#include "coro/SharedMutex.h"
    +#include "coro/Sleep.h"
    +#include "coro/SpinLock.h"
    +#include "coro/SyncAwait.h"
    +#include "executors/SimpleExecutor.h"
    +#include "executors/SimpleIOExecutor.h"
    +#include "uthread/Async.h"
    +#include "uthread/Await.h"
    +#include "uthread/Collect.h"
    +#include "uthread/Latch.h"
    +#include "uthread/Uthread.h"
    +#include "uthread/internal/thread.h"
    +#include "uthread/internal/thread_impl.h"
    +#include "util/Condition.h"
    +#include "util/Queue.h"
    +#include "util/ThreadPool.h"

  • // There are some bugs in clang lower versions.
    #if defined(clang_major) && clang_major >= 17
    #include "coro/Generator.h"
    #endif
    Error: Process completed with exit code 1.

这个格式错误如果要更改的话,我怕影响到本来的先后依赖关系,Clang Format想全部按照字母顺序排。 @ChuanqiXu9 麻烦大佬帮忙看下

这个项目的头文件在 bazel 构建中没有使用 textual_hdrs,可以构建成功至少可以说明对应头文件是自包含的,理论上应该不会出现先后依赖顺序的问题。
如果你确认这里的确有头文件的依赖关系,可以使用 // clang-format off// clang-format on 隔离一下这个代码块。

@fanfanffy163
Copy link
Copy Markdown
Contributor Author

fanfanffy163 commented Mar 16, 2026

The diff you sent is not formatted correctly. The suggested format is diff --git a/async_simple/async_simple.cppm b/async_simple/async_simple.cppm index 35fd598..4957222 100644 --- a/async_simple/async_simple.cppm +++ b/async_simple/async_simple.cppm @@ -51,35 +51,35 @@ export extern "C++" { #if defined(clang_major) && clang_major >= 17 #include "coro/PromiseAllocator.h" #endif

  • #include "coro/Lazy.h"
  • #include "uthread/internal/thread_impl.h"
  • #include "uthread/Await.h"
  • #include "uthread/Latch.h"
  • #include "uthread/internal/thread.h"
  • #include "uthread/Uthread.h"
  • #include "uthread/Async.h"
  • #include "coro/SpinLock.h"
  • #include "coro/ConditionVariable.h"
  • #include "coro/Latch.h"
  • #include "coro/CountEvent.h"
  • #include "coro/Collect.h"
  • #include "IOExecutor.h"
  • #include "coro/SharedMutex.h"
  • #include "uthread/Collect.h"
  • #include "executors/SimpleIOExecutor.h"
  • #include "coro/Mutex.h"
  • #include "Collect.h"
  • #include "util/Condition.h"
  • #include "coro/Dispatch.h"
  • #include "coro/Sleep.h"
  • #include "util/Queue.h"
  • #include "util/ThreadPool.h"
  • #include "coro/ResumeBySchedule.h"
  • #include "coro/FutureAwaiter.h"
  • #include "coro/SyncAwait.h"
  • #include "executors/SimpleExecutor.h"
  • #include "coro/Semaphore.h"
  • // There are some bugs in clang lower versions.
    +#include "Collect.h"
    +#include "IOExecutor.h"
    +#include "coro/Collect.h"
    +#include "coro/ConditionVariable.h"
    +#include "coro/CountEvent.h"
    +#include "coro/Dispatch.h"
    +#include "coro/FutureAwaiter.h"
    +#include "coro/Latch.h"
    +#include "coro/Lazy.h"
    +#include "coro/Mutex.h"
    +#include "coro/ResumeBySchedule.h"
    +#include "coro/Semaphore.h"
    +#include "coro/SharedMutex.h"
    +#include "coro/Sleep.h"
    +#include "coro/SpinLock.h"
    +#include "coro/SyncAwait.h"
    +#include "executors/SimpleExecutor.h"
    +#include "executors/SimpleIOExecutor.h"
    +#include "uthread/Async.h"
    +#include "uthread/Await.h"
    +#include "uthread/Collect.h"
    +#include "uthread/Latch.h"
    +#include "uthread/Uthread.h"
    +#include "uthread/internal/thread.h"
    +#include "uthread/internal/thread_impl.h"
    +#include "util/Condition.h"
    +#include "util/Queue.h"
    +#include "util/ThreadPool.h"
  • // There are some bugs in clang lower versions.
    #if defined(clang_major) && clang_major >= 17
    #include "coro/Generator.h"
    #endif
    Error: Process completed with exit code 1.

这个格式错误如果要更改的话,我怕影响到本来的先后依赖关系,Clang Format想全部按照字母顺序排。 @ChuanqiXu9 麻烦大佬帮忙看下

这个项目的头文件在 bazel 构建中没有使用 textual_hdrs,可以构建成功至少可以说明对应头文件是自包含的,理论上应该不会出现先后依赖顺序的问题。 如果你确认这里的确有头文件的依赖关系,可以使用 // clang-format off// clang-format on 隔离一下这个代码块。

The diff you sent is not formatted correctly. The suggested format is diff --git a/async_simple/async_simple.cppm b/async_simple/async_simple.cppm index 35fd598..4957222 100644 --- a/async_simple/async_simple.cppm +++ b/async_simple/async_simple.cppm @@ -51,35 +51,35 @@ export extern "C++" { #if defined(clang_major) && clang_major >= 17 #include "coro/PromiseAllocator.h" #endif

  • #include "coro/Lazy.h"
  • #include "uthread/internal/thread_impl.h"
  • #include "uthread/Await.h"
  • #include "uthread/Latch.h"
  • #include "uthread/internal/thread.h"
  • #include "uthread/Uthread.h"
  • #include "uthread/Async.h"
  • #include "coro/SpinLock.h"
  • #include "coro/ConditionVariable.h"
  • #include "coro/Latch.h"
  • #include "coro/CountEvent.h"
  • #include "coro/Collect.h"
  • #include "IOExecutor.h"
  • #include "coro/SharedMutex.h"
  • #include "uthread/Collect.h"
  • #include "executors/SimpleIOExecutor.h"
  • #include "coro/Mutex.h"
  • #include "Collect.h"
  • #include "util/Condition.h"
  • #include "coro/Dispatch.h"
  • #include "coro/Sleep.h"
  • #include "util/Queue.h"
  • #include "util/ThreadPool.h"
  • #include "coro/ResumeBySchedule.h"
  • #include "coro/FutureAwaiter.h"
  • #include "coro/SyncAwait.h"
  • #include "executors/SimpleExecutor.h"
  • #include "coro/Semaphore.h"
  • // There are some bugs in clang lower versions.
    +#include "Collect.h"
    +#include "IOExecutor.h"
    +#include "coro/Collect.h"
    +#include "coro/ConditionVariable.h"
    +#include "coro/CountEvent.h"
    +#include "coro/Dispatch.h"
    +#include "coro/FutureAwaiter.h"
    +#include "coro/Latch.h"
    +#include "coro/Lazy.h"
    +#include "coro/Mutex.h"
    +#include "coro/ResumeBySchedule.h"
    +#include "coro/Semaphore.h"
    +#include "coro/SharedMutex.h"
    +#include "coro/Sleep.h"
    +#include "coro/SpinLock.h"
    +#include "coro/SyncAwait.h"
    +#include "executors/SimpleExecutor.h"
    +#include "executors/SimpleIOExecutor.h"
    +#include "uthread/Async.h"
    +#include "uthread/Await.h"
    +#include "uthread/Collect.h"
    +#include "uthread/Latch.h"
    +#include "uthread/Uthread.h"
    +#include "uthread/internal/thread.h"
    +#include "uthread/internal/thread_impl.h"
    +#include "util/Condition.h"
    +#include "util/Queue.h"
    +#include "util/ThreadPool.h"
  • // There are some bugs in clang lower versions.
    #if defined(clang_major) && clang_major >= 17
    #include "coro/Generator.h"
    #endif
    Error: Process completed with exit code 1.

这个格式错误如果要更改的话,我怕影响到本来的先后依赖关系,Clang Format想全部按照字母顺序排。 @ChuanqiXu9 麻烦大佬帮忙看下

这个项目的头文件在 bazel 构建中没有使用 textual_hdrs,可以构建成功至少可以说明对应头文件是自包含的,理论上应该不会出现先后依赖顺序的问题。 如果你确认这里的确有头文件的依赖关系,可以使用 // clang-format off// clang-format on 隔离一下这个代码块。

在普通的 Bazel 构建下它确实是自包含的。但是在 .cppm 模块化构建时,由于开启了 ASYNC_SIMPLE_USE_MODULES 宏,ConditionVariable.h 内部的 #include "SpinLock.h" 被 #ifndef 屏蔽了,导致在 .cppm 内部失去了自包含特性,所以这两个文件产生了先后顺序依赖。

那我再操作一下// clang-format off 和 // clang-format on 隔离一下这个代码块,感谢@c8ef

@ChuanqiXu9 ChuanqiXu9 merged commit 8df816f into alibaba:main Mar 17, 2026
15 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants