Skip to content

fix(lru/server/router): 合并后评审的三项修复 - #474

Merged
adam-ikari merged 1 commit into
mainfrom
fix/review-followups
Oct 13, 2026
Merged

adam-ikari merged 1 commit into
mainfrom
fix/review-followups

Conversation

@adam-ikari

Copy link
Copy Markdown
Owner

对 #471/#470/#472/#473 合并后(main d257c92)做批判性精读后的修复。上一轮的一致性对账全过,这轮是读代码找缺陷,共 5 项发现,本 PR 处理 3 项,另 2 项记录在案。

P1 — uvhttp_lru_cache_set_max_file_size(0) 砖化缓存(真实缺陷,探针实证)

#473 给 lru_cache_set_max_entries / set_max_memory_usage 补了「写点校验」,漏了同文件第三个 setter —— 而这个比那两个更毒:

  • put() 在 lru_cache.c:349 用 content_length > cache->max_file_size 对内容长度做硬截断
  • create() 不接收该参数(默认 1GB 写死在 :82),所以 setter 是这个字段唯一的守卫,而它裸赋值
  • 写 0 之后每次 put 都返回 UVHTTP_ERROR_INVALID_PARAM,缓存永久不可用,调用方只看到一串 File size exceeds limit 的 WARN

评审探针实测:set(0) → max_file_size=0 → put 返回 -1。

当时按「create 拒绝的值 setter 不得接受」扫描,漏它的原因正在于此:它不在 create 的参数表里。修法与另两个同范式(告警 + 保持原值),并补 3 个用例,含核心回归「写入 0 之后 put 仍成功」。变异验证:撤掉校验 → 2 个新用例变红。

P1 — take_router / take_context 的护栏是每 server 的,关不住跨 server 共享

护栏只查「这一个 server 自己的当前指针」,堵住了同一 server 收两次不同对象,但漏了把同一对象交给两个 server:

take_router(A, R); take_router(B, R);
uvhttp_server_free(A); uvhttp_server_free(B);   // 第二次 uvhttp_router_free → double free

context 同理(server_free 对 context 调 destroy)。ASan 只在 free 时报,远离赋值点。

限定:这是既有行为(旧 set_router 完全无护栏),非本轮引入;且本轮把「所有权已移交、不可替换」写成了头文件契约,而契约比护栏宽 —— 所以修的是口径:头文件 @note 明确「一个对象只属于一个 server」。用所有权登记表强制属 d6 范围(会变成破坏性),不在本 PR。

P2 — migrate_to_trie 注释把「会被下次尝试复用」当成已验证事实

失败路径留在池中的半成品节点「被复用」这一主张既无测试也无自然触达路径(触发条件是 12 子节点上限,而本仓无删路由 API,失败后不可能再成功)。改为如实描述:池单调增长直到迁移成功或 router 被释放;retry 按段名匹配复用既有节点 —— 标注「按代码推演,非测试验证」。

本 PR 不修、记录在案的 2 项

  • set_body 的 OOM 原子性无测试牙齿:唯一能区分新旧行为的是分配失败分支,本仓无分配失败注入设施(test_lru_cache_full_coverage.cpp 有同样说明)。需注入基建,单独排期
  • listen_routes 与配置通道不相交:它只调 server_new,config/context 均为 NULL,backlog、tcp_keepalive_timeout 静默走编译期常量。已在 listen_routes 的头文件 @note 写明,结构性解决属 d5/d6

验证

ctest 120/120(默认构建);static-files 配置 120/120(LRU 14 个用例在该配置下真实执行);cppcheck 零告警;test-validity 门禁 PASS;门禁口径内文件 clang-format 18.1.8 合规。

…享、迁移注释冒充已验证

对 #471/#470/#472/#473 合并后(main d257c92)做批判性精读,修复其中三项。
另两项(set_body 的 OOM 覆盖需要分配失败注入设施、listen_routes 与配置
通道相交)分别单独排期与本 PR 的文档注记。

## P1:uvhttp_lru_cache_set_max_file_size(0) 砖化缓存

#473 给 lru_cache_set_max_entries / set_max_memory_usage 补了「写点校验」,
漏了同文件的第三个 setter —— 而这个比那两个更毒:

- put() 在 lru_cache.c:349 用 `content_length > cache->max_file_size` 对内容
  长度做硬截断;create() 又不接收该参数(默认 1GB 写死在 :82)。
- 于是 setter 是该字段唯一守卫,而它裸赋值:写 0 之后**每次 put 都返回
  UVHTTP_ERROR_INVALID_PARAM**,缓存永久不可用,调用方只看到一串
  "File size exceeds limit" 的 WARN。
- 评审探针实测:set(0) → max_file_size=0 → put 返回 -1。

当时按「create 拒绝的值 setter 不得接受」扫描,漏它的原因正在于此:它不在
create 的参数表里。修法与另两个同范式(告警 + 保持原值),并补 3 个用例
(含核心回归「写入 0 之后 put 仍成功」)。变异验证:撤掉校验 →
SetMaxFileSizeRejectsZero / ZeroMaxFileSizeDoesNotBrickTheCache 变红。

## P1:take_router / take_context 的护栏是每 server 的,关不住跨 server 共享

take 的护栏只查「这一个 server 自己的当前指针」,成功堵住同一 server 收
两次不同对象,但漏了把同一对象交给两个 server:

    take_router(A, R); take_router(B, R);
    server_free(A);  server_free(B);   → 第二次 uvhttp_router_free 是 double free

context 同理(server_free 对 context 调 destroy)。ASan 只在 free 时报,
远离赋值点。这是既有行为(旧 set_router 完全无护栏),但本轮把「所有权已
移交、不可替换」写成了头文件契约,而契约比护栏宽 —— 需写明口径。

修法:头文件 @note 明确「一个对象只属于一个 server」。用所有权登记表强制
属 d6 范围(会变成破坏性),不在本 PR。

## P2:migrate_to_trie 注释把「会被下次尝试复用」当成已验证事实

失败路径留在池中的半成品节点「被复用」这一主张,既无测试也无自然触达路径
(触发条件是 12 子节点上限,而本仓无删路由 API,失败后不可能再成功)。改为
如实描述:池单调增长直到迁移成功或 router 释放;retry 会按段名匹配复用既有
节点 —— 标注「按代码推演,非测试验证」。

## 另两项不修、只记录

- set_body 的 OOM 原子性无测试牙齿(唯一能区分新旧的是分配失败分支,本仓无
  注入设施)——单独排期
- listen_routes 建出的 server 不经 context/config,所有限额走编译期常量 — —
  已在头文件 @note 写明,结构性解决属 d5/d6

验证:ctest 120/120(默认构建);static-files 配置 120/120,其中 LRU 14 个
用例真实执行;cppcheck 零告警;test-validity 门禁 PASS;门禁口径内文件
clang-format 18.1.8 合规。
@adam-ikari
adam-ikari merged commit c79d20f into main Oct 13, 2026
10 checks passed
@adam-ikari
adam-ikari deleted the fix/review-followups branch October 13, 2026 01:23
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.

1 participant