Skip to content

feat: minimum implementation of OnceCell - #69

Merged
tisonkun merged 13 commits into
apache:mainfrom
BewareMyPower:bewaremypower/once-lock
Nov 23, 2025
Merged

tisonkun merged 13 commits into
apache:mainfrom
BewareMyPower:bewaremypower/once-lock

Conversation

@BewareMyPower

@BewareMyPower BewareMyPower commented Nov 22, 2025 •

Copy link
Copy Markdown
Contributor

This PR only includes the very essentially asynchronous implementation of OnceCell:

  • new
  • drop
  • get
  • get_or_init

Some differences from tokio:

  • Since mea does not support const version constructors (typically const_new), the OnceCell in mea only supports shared among various tasks via Arc<OnceCell<T>>, while tokio allows the static instance.
  • initialized() is not exposed, the get method should be the alternative to check if the value is initialized with very low overhead (just an construction of Option of a reference)

The key implementation is to use a semaphore whose permits is 1, where each get_or_init call could acquire 1 permit and release it at the end before initialized() returns true. It means there is only 1 task that is able to set the value. Other waiting tasks could be waked by the release operation on Semaphore.

TODO: once this basic implementation is accepted, I will add more methods based on it.

@BewareMyPower BewareMyPower self-assigned this Nov 22, 2025
@BewareMyPower
BewareMyPower marked this pull request as draft November 22, 2025 13:19
@BewareMyPower
BewareMyPower marked this pull request as ready for review November 22, 2025 13:58
@BewareMyPower

Copy link
Copy Markdown
Contributor Author

I adopt a different design with #66 that even if a task acquired the lock and found the value had been initialized, it would still release the semaphore so that other waited get_or_init callers can be notified. While in #66, it calls guard.forget() to skip releasing the semaphore, which looks a bit confusing.

I added some logs and here are some example outputs of multi_init:

worker-2 Acquired semaphore (initialized: false)
worker-2 Released semaphore
worker-2 Acquired semaphore (initialized: true)
worker-2 Released semaphore
worker-0 Acquired semaphore (initialized: false)
worker-0 Released semaphore
worker-0 Acquired semaphore (initialized: true)
worker-0 Released semaphore
worker-0 Acquired semaphore (initialized: true)
worker-0 Released semaphore
worker-0 Acquired semaphore (initialized: true)
worker-0 Released semaphore
worker-0 Acquired semaphore (initialized: true)
worker-0 Released semaphore
worker-0 Acquired semaphore (initialized: true)
worker-0 Released semaphore

It ensures that after each get_or_init is done, the permits is always 1 so that it only allows at most 1 concurrent access before the OnceCell is initialized (just a mutex)


@orthur2 Could you also help take a look?

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR implements a minimal asynchronous OnceCell type that ensures a value is initialized exactly once across multiple concurrent tasks. The implementation uses a semaphore-based synchronization mechanism to guarantee thread-safe lazy initialization.

Key changes:

  • Added OnceCell<T> with new(), get(), and get_or_init() methods using semaphore synchronization
  • Implemented proper memory management with Drop trait for cleanup of initialized values
  • Added test coverage for drop behavior and concurrent initialization scenarios

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
mea/src/once/once_cell.rs Core implementation of OnceCell using semaphore, atomic bool, and unsafe cell for thread-safe lazy initialization
mea/src/once/tests.rs Test cases covering drop semantics and concurrent multi-task initialization
mea/src/once/mod.rs Module declaration exposing the once_cell submodule
mea/src/lib.rs Integration of once module into library exports and compile-time trait checks

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread mea/src/once/tests.rs Outdated
Comment thread mea/src/once/once_cell.rs Outdated
@orthur2

orthur2 commented Nov 22, 2025 •

Copy link
Copy Markdown
Contributor

I adopt a different design with #66 that even if a task acquired the lock and found the value had been initialized, it would still release the semaphore so that other waited get_or_init callers can be notified. While in #66, it calls guard.forget() to skip releasing the semaphore, which looks a bit confusing.我在 #66 中采用了不同的设计:即使某个任务获取了锁并发现值已被初始化,它仍然会释放信号量,以便通知其他等待的 get_or_init 调用者。而在 #66 中,它调用 guard.forget() 来跳过释放信号量的步骤,这看起来有点令人困惑。

I added some logs and here are some example outputs of multi_init:我添加了一些日志,以下是 multi_init 的一些示例输出:

worker-2 Acquired semaphore (initialized: false)
worker-2 Released semaphore
worker-2 Acquired semaphore (initialized: true)
worker-2 Released semaphore
worker-0 Acquired semaphore (initialized: false)
worker-0 Released semaphore
worker-0 Acquired semaphore (initialized: true)
worker-0 Released semaphore
worker-0 Acquired semaphore (initialized: true)
worker-0 Released semaphore
worker-0 Acquired semaphore (initialized: true)
worker-0 Released semaphore
worker-0 Acquired semaphore (initialized: true)
worker-0 Released semaphore
worker-0 Acquired semaphore (initialized: true)
worker-0 Released semaphore

It ensures that after each get_or_init is done, the permits is always 1 so that it only allows at most 1 concurrent access before the OnceCell is initialized (just a mutex)它确保每次执行 get_or_init 操作后,permits 始终为 1,这样在 OnceCell 初始化之前,最多只允许 1 个并发访问(只是一个互斥锁)。

@orthur2 Could you also help take a look?您能否也帮忙看一下?

First of all, I also agree that guard.forget() is problematic. When I planned to fix this PR today, I made a similar change: introducing a helper ptr and removing Guard::forget , since it’s no longer needed. So I think your approach is correct.

Overall, this PR is mostly correct. There are some minor issues, such as inaccurate comments in multiple places — you should use /// and // appropriately.

There are also some minor style-related points, for example moving Send and Sync to the end of the struct to match the project’s existing style.

Finally, I suggest adding a note in the documentation: calling get_or_init recursively will lead to a deadlock.

@BewareMyPower

Copy link
Copy Markdown
Contributor Author

@orthur2 Thanks for your valuable review, I'm addressing the comments (as well as copilot's) soon

@BewareMyPower
BewareMyPower marked this pull request as draft November 23, 2025 05:41
@BewareMyPower
BewareMyPower marked this pull request as ready for review November 23, 2025 06:01
@BewareMyPower

Copy link
Copy Markdown
Contributor Author

@orthur2 Comments should be addressed now, some comment style issues before were mainly related to safety comments. I also added a test for cancellation. PTAL again.

BTW, I found tokio's API comment is wrong. https://docs.rs/tokio/latest/tokio/sync/struct.OnceCell.html#method.get_or_init

If the operation panics, the whole application will panic as well.

@orthur2

orthur2 commented Nov 23, 2025

Copy link
Copy Markdown
Contributor

@orthur2 Comments should be addressed now, some comment style issues before were mainly related to safety comments. I also added a test for cancellation. PTAL again.现在应该处理一下评论了,之前的一些评论格式问题主要与安全评论有关。我还添加了取消功能的测试。请再次查看。

BTW, I found tokio's API comment is wrong. https://docs.rs/tokio/latest/tokio/sync/struct.OnceCell.html#method.get_or_init顺便说一下,我发现 tokio 的 API 注释有误。https ://docs.rs/tokio/latest/tokio/sync/struct.OnceCell.html#method.get_or_init

If the operation panics, the whole application will panic as well.如果操作失败,整个应用程序也会失败。

Looks great.

Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun tisonkun mentioned this pull request Nov 23, 2025
@tisonkun

Copy link
Copy Markdown
Member

Push some commits for code tidy and make WaitList::new a const fn so that the typical static CELL: OnceCell<T> = OnceCell::new() can work.

I'll merge this patch and any further improvement can be made then.

@tisonkun
tisonkun merged commit 7f4fed6 into apache:main Nov 23, 2025
9 checks passed
@BewareMyPower
BewareMyPower deleted the bewaremypower/once-lock branch November 24, 2025 05:33
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