Skip to content

channel minor refactor#98

Merged
wokron merged 14 commits into
masterfrom
update-channel
Apr 27, 2026
Merged

channel minor refactor#98
wokron merged 14 commits into
masterfrom
update-channel

Conversation

@wokron
Copy link
Copy Markdown
Member

@wokron wokron commented Apr 25, 2026

No description provided.

@wokron wokron changed the title channel minor update channel minor refactor Apr 25, 2026
@wokron wokron requested a review from Copilot April 25, 2026 10:11
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors condy::Channel push handling to distinguish move vs copy pushes and adjusts internal queue bookkeeping, with added tests covering cancellation semantics and copy-push behavior.

Changes:

  • Split async push into push(T&&) (move) and push(const T&) (copy) senders and adjusted push finish-handle storage.
  • Removed size_ member and compute size from tail_ - head_ for buffer accounting.
  • Added tests for canceling a move-only push and for verifying the copy-push sender type.

Reviewed changes

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

File Description
include/condy/channel.hpp Refactors push sender/finish-handle internals; changes push overload set; changes size bookkeeping.
tests/test_channel.cpp Adds coverage for canceling move-only push and for copy-push sender selection.
Comments suppressed due to low confidence (1)

include/condy/channel.hpp:375

  • PushFinishHandleBase::schedule() does delete this via a PushFinishHandleBase*. Because PushFinishHandleBase has no virtual destructor, deleting a derived fake handle (FakePushFinishHandle in force_push) through the base type is undefined behavior and will skip destroying the stored item (e.g., leaking std::unique_ptr). Fix by avoiding base-typed deletion (e.g., add a virtual destructor, or add a function-pointer deleter in the base like Invoker does, and call that instead of delete this).
    void schedule() noexcept {
        if (runtime_ == nullptr) [[unlikely]] {
            // Fake handle, no need to schedule
            delete this;
        } else {
            runtime_->schedule(this);
        }

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

Comment thread include/condy/channel.hpp
Comment thread include/condy/channel.hpp
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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


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

Comment thread include/condy/channel.hpp
Comment thread include/condy/channel.hpp
Comment thread tests/test_channel.cpp
Comment thread include/condy/channel.hpp
@wokron wokron marked this pull request as ready for review April 25, 2026 12:41
@wokron wokron merged commit 6c9a8fb into master Apr 27, 2026
11 checks passed
@wokron wokron deleted the update-channel branch April 27, 2026 02:39
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.

2 participants