Skip to content

test: remove TLS handshake-limit counter race from ConnectionAdmissionTests #229

Closed

Description

@SunSi12138

现象

PR #227 的 PR Quick(run 31996872168)在 Unit Tests 中失败:

TlsHandshakeLimitRejectsTheSecondConnectionWhileTheFirstStalls
Expected to be 1
but found 2

at Assert.That(server.ConnectionAdmission.ActiveConnections).IsEqualTo(1)
ConnectionAdmissionTests.cs:520

#227 没有修改 connection admission 实现或该测试;该覆盖来自已合并的 #162 / PR #209。

竞态窗口

当前测试流程:

using var second = new TcpClient();
await second.ConnectAsync(...);
var secondClosed = await ReadUntilClosedAsync(second);
Ensure(secondClosed, ...);
await Assert.That(server.ConnectionAdmission.ActiveConnections).IsEqualTo(1);

但 #162 最终设计明确规定 connection lease 在 terminal cleanup 完成之后才释放,避免慢 dispose 时提前把槽位交给新连接。

因此客户端 ReadUntilClosedAsync(second) 看到 socket 已关闭,并不构成“服务端 terminal cleanup + lease release 已经完成”的同步点。合法时序可以是:

server rejects second
-> closes/disposes transport enough for client to observe EOF
-> client ReadUntilClosedAsync returns
-> server-side terminal cleanup still finishing
-> ActiveConnections temporarily remains 2
-> lease release
-> ActiveConnections becomes 1

PR #227 CI 正好命中了这个窗口。

判断

目前证据更符合 测试同步竞态,不是 admission 产品语义回归:

如果后续证明 ActiveConnections 在 terminal cleanup 完成后仍不归 1,再升级为产品 bug。

建议修复

将即时断言改为等待真正需要的服务端状态,例如:

await YieldUntilAsync(
    () => server.ConnectionAdmission.ActiveConnections == 1,
    "rejected TLS connection must release its connection slot after terminal cleanup");
await Assert.That(server.ConnectionAdmission.ActiveHandshakes).IsEqualTo(1);

同时保留 ReadUntilClosedAsync(second),分别验证:

  1. 超 handshake limit 的连接确实被立即关闭;
  2. terminal cleanup 最终把 connection accounting 收敛到 1。

不要用固定 Task.Delay 掩盖窗口。

回归验证

  • Linux CI 上循环运行该测试(建议 >= 100 次)。
  • 与其它 ConnectionAdmissionTests 并行/全套运行,0 flake。
  • 若能加 test seam,确定性地阻塞 terminal cleanup,在 client 已观察 close、lease 尚未 release 时证明 ActiveConnections == 2 是允许的 transient,再释放 gate 并断言最终归 1。

Activity

  1. SunSi12138 commented on Aug 17, 2026

    @SunSi12138
    OwnerAuthor

    已通过 PR #234 修复并合并(merge commit 51b8157c,head 2d31d9a)。

    根因

    客户端 ReadUntilClosedAsync 观察到 socket 关闭,不等于服务端 terminal cleanup + connection lease release 已经完成(#162 / PR #209 的设计要求 release after terminal cleanup)。因此 ActiveConnections 在该同步点之后仍可能合法地短暂为 2,即时断言 IsEqualTo(1) 断言的是瞬态。

    修复

    ConnectionAdmissionTests.TlsHandshakeLimitRejectsTheSecondConnectionWhileTheFirstStalls 将即时断言改为收敛等待:

    await YieldUntilAsync(
        () => server.ConnectionAdmission.ActiveConnections == 1,
        "rejected TLS connection must release its connection slot after terminal cleanup");

    保留了 ReadUntilClosedAsync(second) 验证"超 handshake limit 的连接被立即关闭",未引入固定 Task.Delay。可选的 test seam 未加:ConnectionSlotStaysHeldUntilTerminalCleanupCompletes 已通过 dispose gate 确定性覆盖 release-after-terminal-cleanup 语义。

    验证证据

    Closes #229。

  2. SunSi12138 commented on Aug 17, 2026

    @SunSi12138
    OwnerAuthor

    Fixed by #234 (merged as 51b8157).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions