Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable strict concurrency checking for NIOTLS #3112

Merged
merged 2 commits into from
Feb 7, 2025

Conversation

rnro
Copy link
Contributor

@rnro rnro commented Feb 7, 2025

Motivation:

To ensure NIOTLS concurrency safety.

Modifications:

  • Enable strict concurrency checking in the package manifest.
  • Require NegotiationResult to be Sendable
    • In NIOTypedApplicationProtocolNegotiationHandler<NegotiationResult> the
      result is used to fulfil promises and perform hops so we need it to be
      sendable when accessed from these other concurrency domains.

Result:

Builds will warn and CI will fail if regressions are introduced.

@rnro rnro force-pushed the strict_concurrency_niotls branch from 143fc69 to 1104b21 Compare February 7, 2025 16:24
rnro added 2 commits February 7, 2025 16:24
Motivation:

To preserve NIOTLS concurrency safety.

Modifications:

Enable strict concurrency checking in the package manifest.

Result:

Builds will warn and CI will fail if regressions are introduced.
In NIOTypedApplicationProtocolNegotiationHandler<NegotiationResult> the
result is used to fulfil promises and perform hops so we need it to be
sendable when accessed from these other concurrency domains.
@rnro rnro force-pushed the strict_concurrency_niotls branch from 1104b21 to 06bc720 Compare February 7, 2025 16:25
@rnro rnro added the 🔨 semver/patch No public API change. label Feb 7, 2025
@rnro rnro marked this pull request as ready for review February 7, 2025 16:32
Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

Nice, this LGTM.

@Lukasa Lukasa merged commit c51907a into apple:main Feb 7, 2025
32 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants