Repository navigation
fix(response/cache): set_body 原子替换 + LRU setter 在写点建立不变量 - #473
Merged
Merged
Conversation
v2.10 roadmap d4。两条同源问题:写路径没有原子性,写入点没有建立不变量。 ## 1. uvhttp_response_set_body 不是原子的(response.c) 原顺序是 free 旧 body → alloc 新 body → memcpy。分配失败时返回 OUT_OF_MEMORY,而旧值已被释放、body_length 被清零:调用方拿到一个空响应, 既无法回退也没有重试素材 —— 一个"补充资源"式的失败路径,却毁掉了已成立 的状态。 改为 allocate-then-swap:新缓冲先到手,旧缓冲才变得可弃。 set_header 免费具备同样的性质(uvhttp_realloc 失败时保留旧块),set_body 自己分配,必须显式买下这个性质 —— 两个 setter 的失败语义此前不一致。 ## 2. LRU cache setter 与 create 对同一不变量给出相反答案(lru_cache.c) - uvhttp_lru_cache_create 拒绝 max_memory_usage == 0("cannot be zero")、 要求 max_entries > 0;淘汰路径把 "> 0" 读作"限额生效"。 - 但 setter 直接赋值:0 会被照单全收,把运行中缓存的内存/条目上限静默关闭。 即"create 拒绝的值,setter 收下并顺手解除限额"。 现在 setter 越界时告警并保持原值 —— 与同文件 set_batch_eviction_size 的 既有范式一致(不变量在写入点建立,而非只在构造点)。 gzip cache 的同名 setter 未动:gzip_cache_set_max_entries / _set_max_memory_usage 本就有下限校验,而 set_cache_ttl 的负值=永不过期 是有测试的既定语义(test_gzip_cache.cpp:184),不是需要拦的越界值。 唯一生产调用方 uvhttp_static_set_cache_config 已在调用点做了 >0 / >=0 守卫,本次是纵深防御,不改变其行为。 ## 测试 test_lru_cache_capacity 新增 6 个用例(变异验证:撤掉两处校验 → 4 个变红, 含"写入 0 不会放宽预算""关闭条目限额会让有界缓存退化成无界"): - set_max_memory_usage(0) 被拒且预算不变 - set_max_entries(0/-5) 被拒 - 正值仍然立即生效 test_response_full_coverage_enhanced 新增 2 个用例,锁住**可达**的失败路径 (非法参数不得动已有 body;二次设置是整体替换)。 两处诚实标注: - set_body 的 OOM 分支**无法被现有手段触发**(本仓没有分配失败注入设施, test_lru_cache_full_coverage.cpp:1104 有同样说明)。修复按构造成立,不靠 测试兜底;新增用例只钉语义,不假装覆盖 OOM。 - 新测试一度用 EXPECT_STREQ 比较 body,被 ASan 判为越界读 —— body 按设计 不带 NUL 终止符(库内一律 body_length + memcpy),已改为按长度 memcmp。 同理撤掉了"二次设置后指针必然改变"的断言:分配器会复用刚释放的块,那是 在测分配器而非语义(旧实现在该断言上同样会红)。 验证:ctest 120/120;新增用例在 ASan 下 27/27 + 11/11 干净;cppcheck 零告警; test-validity 门禁 PASS;改动文件 clang-format 18.1.8 合规。
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.
两个同源问题:写路径没有原子性,写入点没有建立不变量
1.
uvhttp_response_set_body不是原子的(response.c)原顺序
free 旧 body → alloc 新 body → memcpy。分配失败时返回OUT_OF_MEMORY,而旧值已被释放、body_length被清零:调用方拿到一个空响应,既无法回退也没有重试素材 —— 一个"补充资源"式的失败路径,却毁掉了已成立的状态。改为 allocate-then-swap:新缓冲先到手,旧缓冲才变得可弃。
set_header免费具备同一性质(uvhttp_realloc失败时保留旧块),set_body自己分配,必须显式买下这个性质 —— 两个 setter 的失败语义此前不一致。2. LRU cache setter 与 create 对同一不变量给出相反答案(lru_cache.c)
uvhttp_lru_cache_create拒绝max_memory_usage == 0("cannot be zero")、要求max_entries > 0;淘汰路径把> 0读作"限额生效"。create拒绝的值,setter 收下并顺手解除了运行中缓存的限额(0 → 视为无限额)。现在 setter 越界时告警并保持原值,与同文件
uvhttp_lru_cache_set_batch_eviction_size的既有范式一致 —— 不变量在写入点建立,而非只在构造点。gzip cache 的同名 setter 未动:
set_max_entries/set_max_memory_usage本就有下限校验;set_cache_ttl的负值 = 永不过期是有测试的既定语义(test_gzip_cache.cpp:184),不是需要拦的越界值。唯一生产调用方uvhttp_static_set_cache_config已在调用点做>0/>=0守卫,本次是纵深防御,不改变其行为。测试
test_lru_cache_capacity+6 个用例。变异验证:撤掉两处校验 → 4 个变红,含"写入 0 不会放宽预算"(0.9×16KB 阈值下的 put 行为不变)与"关闭条目限额会让有界缓存退化成无界"(entry_count <= max_entries仍成立)test_response_full_coverage_enhanced+2 个用例,锁住可达的失败路径:非法参数不得动已有 body;二次设置是整体替换两处诚实标注(不假装覆盖)
test_lru_cache_full_coverage.cpp:1104有同样说明),修复按构造成立、不靠测试兜底EXPECT_STREQ比较 body 被 ASan 判为越界读 —— body 按设计不带 NUL 终止符(库内一律body_length+memcpy),已改为按长度memcmp。同理撤掉了"二次设置后指针必然改变"的断言:分配器会复用刚释放的块,那是在测分配器而非语义(旧实现在该断言上同样会红)验证
ctest 119/119(static-files 开启配置,
test_lru_cache_capacity的 11 个用例真实执行);新增用例在 ASan 下 27/27 + 11/11 干净;cppcheck 零告警;test-validity 门禁 PASS。