Skip to content

Pr/capy update#215

Merged
mvandeberg merged 2 commits intocppalliance:developfrom
mvandeberg:pr/capy-update
Mar 20, 2026
Merged

Pr/capy update#215
mvandeberg merged 2 commits intocppalliance:developfrom
mvandeberg:pr/capy-update

Conversation

@mvandeberg
Copy link
Contributor

@mvandeberg mvandeberg commented Mar 20, 2026

Summary by CodeRabbit

  • Chores

    • Updated Apple Clang compiler configuration to target macOS 15 and removed duplicate build matrix entries.
  • Bug Fixes

    • Fixed signal handler cancellation behavior to return properly structured result values.
    • Enhanced mutex lock management in TLS stream implementations (OpenSSL and WolfSSL) to improve thread-safe I/O operation handling.

@coderabbitai
Copy link

coderabbitai bot commented Mar 20, 2026

Warning

Rate limit exceeded

@mvandeberg has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 23 minutes and 34 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d39518ce-f885-43e9-9c77-4d5309054468

📥 Commits

Reviewing files that changed from the base of the PR and between 5a8e46a and 973befd.

📒 Files selected for processing (3)
  • .github/compilers.json
  • src/openssl/src/openssl_stream.cpp
  • src/wolfssl/src/wolfssl_stream.cpp
📝 Walkthrough

Walkthrough

This PR updates the apple-clang compiler CI target to macOS 15, standardizes signal set error return values to include a signal number field, and refactors async mutex locking patterns across TLS stream implementations to use explicit lock guards instead of scoped locks.

Changes

Cohort / File(s) Summary
CI Configuration
.github/compilers.json
Updated apple-clang matrix entry from macos-14 to macos-15 and removed a duplicate apple-clang entry.
Signal Set Error Returns
include/boost/corosio/io/io_signal_set.hpp, include/boost/corosio/native/native_signal_set.hpp
Modified cancellation error returns to include a second field (0 for signal number) in the capy::io_result<int>, changing from {capy::error::canceled} to {capy::error::canceled, 0}.
TLS Stream Mutex Locking Refactor
src/openssl/src/openssl_stream.cpp, src/wolfssl/src/wolfssl_stream.cpp
Refactored async mutex acquisition pattern: replaced co_await io_cm_.scoped_lock() with co_await io_cm_.lock() followed by explicit capy::async_mutex::lock_guard construction, applied across multiple I/O paths including read, write, handshake, and shutdown operations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 A mutex lock doth change its form,
From scoped guards to guards explicit,
The signals now speak loud and clear—
With numbers paired, the flow's polished,
And macOS fifteen takes the stage! 🎭

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Pr/capy update' is vague and generic, using non-descriptive terminology that fails to convey meaningful information about the specific changes made in the pull request. Provide a more specific title that describes the primary changes, such as 'Update signal set and TLS stream implementations for async mutex API' or 'Refactor async mutex locking in signal sets and OpenSSL/WolfSSL streams'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cppalliance-bot
Copy link

cppalliance-bot commented Mar 20, 2026

An automated preview of the documentation is available at https://215.corosio.prtest3.cppalliance.org/index.html

If more commits are pushed to the pull request, the docs will rebuild at the same URL.

2026-03-20 23:51:58 UTC

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/wolfssl/src/wolfssl_stream.cpp`:
- Around line 887-890: The lock acquisition error from awaiting io_cm_.lock() is
being swallowed: when auto [lec] = co_await io_cm_.lock(); yields an error the
code simply breaks without propagating it; update the loop handling around
io_cm_ to set ec = lec before breaking (mirror the pattern used at the earlier
block that assigns ec = lec) so the function returns the lock error instead of
success; locate the await of io_cm_.lock() and the subsequent if (lec) break;
and insert assignment of ec = lec immediately before the break.
- Around line 911-914: The lock acquisition result from co_await io_cm_.lock()
is being ignored when lec is non-null (the code does "if (lec) break;"),
silently swallowing the error; instead propagate the lock error to the caller
(do not break out locally). Replace the break with code that forwards the error
(e.g., co_return or return the error code/value, or rethrow) so the caller of
the coroutine/function sees the lock failure; update the logic around the
io_cm_.lock() handling (the variable lec and the surrounding coroutine) to
return/propagate the error rather than exiting the loop silently.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e45a2919-9371-4b8e-8c64-5eb7d7e1629f

📥 Commits

Reviewing files that changed from the base of the PR and between 07fab88 and 5a8e46a.

⛔ Files ignored due to path filters (1)
  • test/unit/io_context.cpp is excluded by !**/test/**
📒 Files selected for processing (5)
  • .github/compilers.json
  • include/boost/corosio/io/io_signal_set.hpp
  • include/boost/corosio/native/native_signal_set.hpp
  • src/openssl/src/openssl_stream.cpp
  • src/wolfssl/src/wolfssl_stream.cpp

@mvandeberg mvandeberg merged commit 5a40e9a into cppalliance:develop Mar 20, 2026
39 of 40 checks passed
@mvandeberg mvandeberg deleted the pr/capy-update branch March 20, 2026 23:53
@codecov
Copy link

codecov bot commented Mar 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.90%. Comparing base (07fab88) to head (973befd).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #215      +/-   ##
===========================================
+ Coverage    77.75%   77.90%   +0.15%     
===========================================
  Files           96       96              
  Lines         7452     7437      -15     
  Branches      1836     1822      -14     
===========================================
  Hits          5794     5794              
+ Misses        1118     1114       -4     
+ Partials       540      529      -11     
Files with missing lines Coverage Δ
include/boost/corosio/io/io_signal_set.hpp 96.15% <ø> (+3.56%) ⬆️
include/boost/corosio/native/native_signal_set.hpp 100.00% <ø> (ø)
src/openssl/src/openssl_stream.cpp 56.30% <ø> (+0.24%) ⬆️
src/wolfssl/src/wolfssl_stream.cpp 62.20% <ø> (+1.32%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 07fab88...973befd. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

3 participants