fix(mvc): 在途请求数无上界——闸门在「派发」处释放,而不是在「完成」处 - #26
Open
Cyaim wants to merge 1 commit into
Open
Conversation
接收循环把请求交给 ProcessMessageAsync 之后,在**同一轮迭代的 finally** 里就把并发闸门 还了回去。也就是说闸门数的是「派发出去几条」而不是「有几条还在跑」。 不等响应就流水线发送的客户端因此完全不受约束:每条在途都持有一个 DI Scope、 一个反射构造的控制器实例和一份解析好的请求体,而没有任何东西在数它们。 在途数 ≈ 攻击者带宽 × 后端时延,而且是正反馈——压力让后端更慢,更慢就有更多在途。 同一个缺陷有两处:连接级的 ParallelForwardLimitSlim 和端点级的 endPointSlim, 都是派发处获取、迭代 finally 释放。 ## 那个闸门还不是「每连接」的 字段的注释写着 "Associated with the connection",而 AddMvcChannel 每通道只建**一个** handler(WebSocketServerBuilder.cs:119 的 new MvcChannelHandler(...).ConnectionEntry), 把同一个委托交给每条连接。于是它名为每连接、实为整条通道共用。 有人以为它是每连接的、把上限设成一个小数字,实际效果是把整个进程串行化—— 与名字承诺的正好相反,而且服务器扛的连接越多越糟。 ## 改动 - 闸门改为 MvcForward 里的局部变量,真正每连接一个;派发前获取,**完成时**释放。 - 交接用一个 gatesOwnedByTask 标志:goto CONTINUE_RECEIVE 那几条路径根本不派发, 票仍归本轮的 finally,不能漏。 - 刻意不 Dispose 那个信号量:连接结束时仍在途的请求会在续体里 Release 它, 在这里 Dispose 会把一次正常断连变成线程池上的 ObjectDisposedException。 - MaxConnectionParallelForwardLimit 不设时不再等于「没有闸门」,默认 16。 - ParallelForwardLimitSlim 标记 [Obsolete] 并说明它为什么是进程级的。 ## 差点变成死锁的那一步 续体的 state 我一开始写成 (logger, inflight, endpoint),而 logger 的静态类型是 ILogger<WebSocketRouteMiddleware>——装箱的是 ValueTuple<ILogger<WebSocketRouteMiddleware>,…>, 而值元组拆箱要求类型**完全一致**,于是续体里那次强转抛 InvalidCastException, 被续体吞掉,票一张都还不回来。一个悄悄变成死锁的安全修复。 是 Finished_requests_give_their_permit_back 抓到的,现在强转成 (ILogger)logger。 ## 测试 InflightRequestLimitTests 4 条,跑真的 TestServer + 中间件 + 接收循环: 上限生效、票会还、不设上限也有界、且界是**每连接**而不是每进程。 两条脚手架陷阱写进了注释,因为它们都让测试静默地失去意义: - 端点键是「类名去掉 Controller」+「.」+ 方法名全小写。写错不会响亮失败: 零个处理器执行,而「peak <= limit」在 peak = 0 时照样通过。已加反真空断言。 - WebSocketRouteOption.ApplicationServices 是静态的:一条在自己 host 消失后还在排空 流水线消息的接收循环,会从**当前**那个 host 解析控制器。现象是后一条测试数到的请求 比它发出去的还多(发 16、数到 20),读起来就像上限失效。每条测试收尾关掉自己的 socket。 全套 710/710 零跳过(706 + 4);下游 im-cloud 构建 0 错 0 警告、单测 2166/2166。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnbKMBNERxHPawj6mVfmEM
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
接收循环把请求交给
ProcessMessageAsync之后,在同一轮迭代的 finally 里就把并发闸门还了回去。闸门数的是「派发出去几条」,不是「有几条还在跑」。不等响应就流水线发送的客户端因此完全不受约束:每条在途都持有一个 DI Scope、一个反射构造的控制器实例和一份解析好的请求体,而没有任何东西在数它们。在途数 ≈ 攻击者带宽 × 后端时延,而且是正反馈——压力让后端更慢,更慢就有更多在途。
同一个缺陷有两处:连接级的
ParallelForwardLimitSlim和端点级的endPointSlim。那个闸门还不是「每连接」的
字段注释写着
Associated with the connection,而AddMvcChannel每通道只建一个 handler(WebSocketServerBuilder.cs:119),把同一个委托交给每条连接。于是它名为每连接、实为整条通道共用。有人以为它是每连接的、把上限设成一个小数字,实际效果是把整个进程串行化——与名字承诺的正好相反,而且服务器扛的连接越多越糟。
改动
MvcForward里的局部变量,真正每连接一个;派发前获取、完成时释放gatesOwnedByTask标志:goto CONTINUE_RECEIVE那几条路径根本不派发,票仍归本轮 finallyMaxConnectionParallelForwardLimit不设时不再等于「没有闸门」,默认 16ParallelForwardLimitSlim标记[Obsolete],并说明它为什么是进程级的差点变成死锁的那一步
续体的 state 我一开始写成
(logger, inflight, endpoint)。logger的静态类型是ILogger<WebSocketRouteMiddleware>,所以装箱的是ValueTuple<ILogger<WebSocketRouteMiddleware>, …>,而值元组拆箱要求类型完全一致——续体里那次强转抛InvalidCastException,被续体吞掉,票一张都还不回来。一个悄悄变成死锁的安全修复。是
Finished_requests_give_their_permit_back抓到的。测试
InflightRequestLimitTests4 条,跑真的 TestServer + 中间件 + 接收循环:上限生效、票会还、不设上限也有界、且界是每连接而不是每进程。两条脚手架陷阱写进了注释,因为它们都让测试静默地失去意义:
peak <= limit在peak = 0时照样通过。已加反真空断言。WebSocketRouteOption.ApplicationServices是静态的:一条在自己 host 消失后还在排空流水线消息的接收循环,会从当前那个 host 解析控制器。现象是后一条测试数到的请求比它发出去的还多(发 16、数到 20),读起来就像上限失效。每条测试收尾关掉自己的 socket。读数
发现方式
六维度并行安全审查里站住的三条 DoS 之一(F2)。审查还纠正了原报的一个方向性错误:把并发数配成 1 不是「拦不住」,是会把整个进程串死——按原报建议改会更糟。