Skip to content

Conversation

@rnro
Copy link
Contributor

@rnro rnro commented Mar 6, 2025

Motivation:

To ensure NIOTestUtils concurrency safety.

Modifications:

  • Enable strict concurrency checking in the package manifest.
  • NIOHTTP1TestServer does pipeline configuration as synchronous operations.

Result:

NIOTestUtils does not warn of concurrency issues, builds will warn and CI will fail if regressions are introduced.

@rnro rnro force-pushed the strict_concurrency_niotestutils branch from 2b29729 to dbbb75b Compare March 6, 2025 09:36
@rnro rnro added the 🔨 semver/patch No public API change. label Mar 6, 2025
@rnro rnro force-pushed the strict_concurrency_niotestutils branch 3 times, most recently from 61bd820 to c0ff3e4 Compare March 6, 2025 17:24
Motivation:

To ensure `NIOTestUtils` concurrency safety.

Modifications:

* Enable strict concurrency checking in the package manifest.
* `NIOHTTP1TestServer` does pipeline configuration as synchronous
  operations.

Result:

`NIOTestUtils` does not warn of concurrency issues, builds will warn and CI will fail if regressions are introduced.
@rnro rnro force-pushed the strict_concurrency_niotestutils branch from c0ff3e4 to bdc82eb Compare March 6, 2025 17:26
@rnro rnro marked this pull request as ready for review March 6, 2025 17:32
@rnro rnro enabled auto-merge (squash) March 6, 2025 17:32
@rnro rnro requested a review from Lukasa March 7, 2025 08:47
try channel.pipeline.syncOperations.addHandler(TransformerHandler())
_ = try channel.syncOptions!.setOption(.autoRead, value: true)
} catch {
assertionFailure("Channel initialization failed with: \(error)")
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's just fully fatalError here, no reason to be shy.

@rnro rnro merged commit 6adf682 into apple:main Mar 7, 2025
35 checks passed
@rnro rnro deleted the strict_concurrency_niotestutils branch March 7, 2025 11:11
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