Skip to content

解决并发 xhr 的 session rule 数量问题#1353

Merged
CodFrm merged 8 commits intomainfrom
pr/fix-multiple-xhr
Apr 23, 2026
Merged

解决并发 xhr 的 session rule 数量问题#1353
CodFrm merged 8 commits intomainfrom
pr/fix-multiple-xhr

Conversation

@cyfung1031
Copy link
Copy Markdown
Collaborator

@cyfung1031 cyfung1031 commented Apr 17, 2026

Checklist / 检查清单

  • Fixes mentioned issues / 修复已提及的问题
  • Code reviewed by human / 代码通过人工检查
  • Changes tested / 已完成测试

Description / 描述

Screenshots / 截图

Screenshot 2026-04-18 at 5 02 22

@cyfung1031 cyfung1031 added the P1 🔥 重要但是不紧急的内容 label Apr 22, 2026
@CodFrm CodFrm requested a review from Copilot April 22, 2026 16:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

该 PR 旨在修复 GM_xmlhttpRequest 在高频并发请求下触发 Session rule count exceeded 的问题,通过更及时地清理 DNR session rules、并引入 session rule id 的集中分配与限额控制,降低规则堆积导致请求被拦截的概率(对应关闭 #1352)。

Changes:

  • GMApi 的 webRequest 生命周期中新增更早的清理时机(onResponseStarted)并补齐 API 出错时的清理逻辑,避免 DNR session rule 长时间滞留。
  • 新增 dnr_id_controller:从现有 session rules 初始化、分配/回收 per-request rule id,并尝试在接近上限时阻塞创建。
  • 更新 chrome mock(DNR session rules 存储、webRequest 事件占位)并新增 dnr_id_controller 单测。

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
tests/vitest.setup.ts 增加(注释掉的)spyOn 相关代码,疑似用于调试 DNR 调用
src/app/service/service_worker/gm_api/gm_api.ts GM XHR 的 DNR rule 分配改为集中控制;新增 headersSettled/cleanup;增加 onResponseStarted 以更早回收规则
src/app/service/service_worker/gm_api/dnr_id_controller.ts 新增 session rule id 跟踪、分配与限额阻塞逻辑
src/app/service/service_worker/gm_api/dnr_id_controller.test.ts 新增对 id 分配/回收/限额阻塞的单测
packages/chrome-extension-mock/web_reqeuest.ts mock 增加 onResponseStarted 入口(但目前是空实现)
packages/chrome-extension-mock/declarativ_net_request.ts mock 增加 session rules 存储、getSessionRules 与 updateSessionRules 的增删逻辑

},
respOpt
);

Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

新增的 handlerGmXhr()/headersSettled/onResponseStarted 逻辑会影响 DNR session rule 的创建与回收(本 PR 的核心修复点),但当前 gm_api.test.ts 仅覆盖 getConnectMatched,未覆盖这些关键路径。建议补充单测:模拟并发请求、触发 onResponseStarted/onErrorOccurred,并断言 updateSessionRules(removeRuleIds) 被调用且 rule id 被正确释放(不会再次触发 session rule count exceeded)。

Suggested change
chrome.webRequest.onErrorOccurred.addListener(
(details) => {
const lastError = chrome.runtime.lastError;
if (lastError) {
console.error("chrome.runtime.lastError in chrome.webRequest.onErrorOccurred:", lastError);
return undefined;
}
if (details.tabId === -1) {
// 请求异常结束时不会再进入 onResponseStarted,这里必须兜底清理
cleanupOnAPIError(details.requestId);
}
},
{
urls: ["<all_urls>"],
types: ["xmlhttprequest"],
}
);

Copilot uses AI. Check for mistakes.
Comment on lines +133 to +146
const lockedPromise = nextSessionRuleId();

const raceResult1 = await Promise.race([lockedPromise.then(() => "resolved"), sleep(5).then(() => "pending")]);
expect(raceResult1).toBe("pending");

const m1 = Math.floor(Math.random() * (added.length - 9));
const p1 = added[m1];
const p2 = added[m1 + 6];
removeSessionRuleIdEntry(p1);
removeSessionRuleIdEntry(p2);

const raceResult2 = await Promise.race([lockedPromise.then(() => "resolved"), sleep(5).then(() => "pending")]);
expect(raceResult2).toBe("resolved");

Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

这些断言依赖非常短的 sleep(5)/sleep(50) 来判断 Promise 是否 pending/resolved,时间敏感且在不同机器负载下容易抖动。建议使用可控的 deferred/fake timers(vi.useFakeTimers/advanceTimersByTime)或明确的同步钩子来判断“是否被锁住/何时解锁”,避免时间窗口导致的偶发失败。

Copilot uses AI. Check for mistakes.
Comment on lines +47 to +56
export const nextSessionRuleId = async () => {
const ruleIds = await getSessionRuleIds();
if (!lockSessionRuleCreation && ruleIds.size + 1 > LIMIT_SESSION_RULES) lockSessionRuleCreation = deferred<void>();
if (lockSessionRuleCreation) await lockSessionRuleCreation.promise;
let id;
do {
id = ++SESSION_RULE_ID_BEGIN;
} while (ruleIds.has(id));
ruleIds.add(id);
return id;
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

dnr_id_controller 的限额锁逻辑在并发场景下会失效:当 rules 已接近/达到 LIMIT_SESSION_RULES 时,多个并发 nextSessionRuleId() 都会在同一个 lock 被 resolve 后同时继续分配并 ruleIds.add(id),从而一次性超出上限(释放 1 个 slot,却可能放行 N 个 waiter)。建议把“等待-检查-分配-计数”做成互斥/队列(例如 mutex + while 循环反复检查 size<limit),或实现计数信号量按释放 slot 数量逐个放行。

Copilot uses AI. Check for mistakes.
Comment on lines +35 to +42
if (sessionRuleIds.delete(ruleId) === true) {
if (ruleId <= SESSION_RULE_ID_BEGIN + 1) {
SESSION_RULE_ID_BEGIN = ruleId - 1;
}
if (sessionRuleIds.size < LIMIT_SESSION_RULES) {
lockSessionRuleCreation?.resolve();
lockSessionRuleCreation = null;
}
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

removeSessionRuleIdEntry() 一次 remove 后会直接 resolve 并把 lockSessionRuleCreation 置空(size<LIMIT 即解锁)。结合 nextSessionRuleId() 当前实现,这会让所有等待者同时继续,无法保证“解锁次数=可用 slot 数”。如果后续按信号量/队列修复并发,这里也需要配套改为按 slot 数逐个唤醒,或在 nextSessionRuleId 内部重新判定并在不足时再次进入等待。

Copilot uses AI. Check for mistakes.
() => {
const lastError = chrome.runtime.lastError;
if (lastError) {
console.error("chrome.declarativeNetRequest.updateSessionRules:", lastError);
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

headersSettled() 在 updateSessionRules 回调里即使检测到 chrome.runtime.lastError 也仍然调用 removeSessionRuleIdEntry(ruleID)。一旦 removeSessionRules 实际失败(规则仍留在浏览器里),本地却释放并复用该 ruleID,后续 updateSessionRules(addRules) 可能会“更新”到旧规则或再次触发上限/冲突,导致更难排查的问题。建议仅在无 lastError 时再 removeSessionRuleIdEntry;若失败则应保留该 id,或重新 getSessionRules() 做一次同步/重建。

Suggested change
console.error("chrome.declarativeNetRequest.updateSessionRules:", lastError);
console.error("chrome.declarativeNetRequest.updateSessionRules:", lastError);
return;

Copilot uses AI. Check for mistakes.
Comment on lines +138 to +140
const m1 = Math.floor(Math.random() * (added.length - 9));
const p1 = added[m1];
const p2 = added[m1 + 6];
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

单测里使用 Math.random() 选择要 remove 的 rule id(m1/p1/p2)会引入非确定性,可能导致 CI 偶发失败且难以复现。建议改为固定索引/构造明确的待删除 ID(例如取 added 的前几个/特定位置),保证测试可重复。

Suggested change
const m1 = Math.floor(Math.random() * (added.length - 9));
const p1 = added[m1];
const p2 = added[m1 + 6];
const p1 = added[0];
const p2 = added[6];

Copilot uses AI. Check for mistakes.
Comment on lines +18 to +22
onResponseStarted = {
addListener: () => {
// TODO
},
};
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

chrome-extension-mock 的 webRequest.onResponseStarted.addListener 目前是空实现,无法捕获并保存回调,这会让后续想在单测里模拟 onResponseStarted 事件变得不可能(而本 PR 正好新增了对此事件的依赖)。建议参考 onBeforeSendHeaders 的实现,把 callback 存下来并提供触发途径(哪怕是最小实现),以便测试覆盖新的清理逻辑。

Copilot uses AI. Check for mistakes.
Comment thread tests/vitest.setup.ts Outdated
Comment on lines +12 to +13
// vi.spyOn(chromeMock.declarativeNetRequest, "getSessionRules");
// vi.spyOn(chromeMock.declarativeNetRequest, "updateSessionRules");
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

vitest.setup.ts 里新增的两行 vi.spyOn(...) 被注释掉后长期保留会增加噪音,且容易让人误以为需要手动开启。若不再需要建议删除;若是调试开关建议改成受环境变量控制并配套说明。

Suggested change
// vi.spyOn(chromeMock.declarativeNetRequest, "getSessionRules");
// vi.spyOn(chromeMock.declarativeNetRequest, "updateSessionRules");

Copilot uses AI. Check for mistakes.
Comment on lines +184 to +189
// Single removal should unlock all waiters sequentially
const toRemove = [...ids].find((id) => id > 10000)!;
removeSessionRuleIdEntry(toRemove);

// All three should eventually resolve
const results = await Promise.race([Promise.all([p1, p2, p3]).then((ids) => ids), sleep(50).then(() => null)]);
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

这个用例假设“只 remove 1 个 id 也应解锁所有并发 nextSessionRuleId() 调用并最终全部 resolve”。如果 nextSessionRuleId 的限额控制要严格保证不超过 LIMIT_SESSION_RULES,那么单次释放的 slot 数应当限制可继续分配的 waiter 数;否则会在高并发下再次触发 session rule count exceeded。建议测试期望与实现一起调整为:释放多少 slot,就最多放行多少个 waiter(其余继续等待)。

Suggested change
// Single removal should unlock all waiters sequentially
const toRemove = [...ids].find((id) => id > 10000)!;
removeSessionRuleIdEntry(toRemove);
// All three should eventually resolve
const results = await Promise.race([Promise.all([p1, p2, p3]).then((ids) => ids), sleep(50).then(() => null)]);
// 单次只释放 1 个 slot,因此最多只能放行 1 个 waiter
const removable = [...ids].filter((id) => id > 10000);
removeSessionRuleIdEntry(removable[0]);
const firstResolved = await Promise.race([p1.then(() => "resolved"), sleep(50).then(() => "pending")]);
expect(firstResolved).toBe("resolved");
const remainingStillPending = await Promise.race([
Promise.all([p2, p3]).then(() => "resolved"),
sleep(5).then(() => "pending"),
]);
expect(remainingStillPending).toBe("pending");
// 继续释放 2 个 slot,剩余 waiter 才能继续完成
removeSessionRuleIdEntry(removable[1]);
removeSessionRuleIdEntry(removable[2]);
const results = await Promise.race([Promise.all([p2, p3]).then((ids) => ids), sleep(50).then(() => null)]);

Copilot uses AI. Check for mistakes.
@CodFrm
Copy link
Copy Markdown
Member

CodFrm commented Apr 22, 2026

Code review

Found 1 issue:

  1. 新增测试文件 dnr_id_controller.test.tsdescribe / it 标题全部为英文,与仓库约定不符。CLAUDE.md 在 "Development Workflow — TDD/BDD First" 一节明确要求:"Use BDD-style descriptions — Test titles in Chinese describing behavior (use describe / it)"(见 CLAUDE.md)。现有测试文件(如 locales.test.tscache.test.tsmessage_queue.test.ts)均遵循此约定。

describe("getSessionRuleIds", () => {
it("initializes from existing chrome session rules", async () => {
const ids = await getSessionRuleIds();
expect(ids.size).lessThan(100);
await nextSessionRuleId();
expect(ids.size).greaterThanOrEqual(1);
await nextSessionRuleId();
expect(ids.size).greaterThanOrEqual(2);
});
});
describe("nextSessionRuleId", () => {
it("returns unique incrementing IDs on each call", async () => {
const id1 = await nextSessionRuleId();
const id2 = await nextSessionRuleId();
const id3 = await nextSessionRuleId();
expect(id2).toBeGreaterThan(id1);
expect(id3).toBeGreaterThan(id2);
});
it("skips IDs that already exist in session rules", async () => {
const ids = await getSessionRuleIds();
const next = await nextSessionRuleId();
ids.add(next + 1);
const skipped = await nextSessionRuleId();
expect(skipped).toBeGreaterThan(next + 1);
});
});
describe("removeSessionRuleIdEntry", () => {
it("removes the given ID from the tracked set", async () => {
const ids = await getSessionRuleIds();
const id = await nextSessionRuleId();

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

Comment on lines +161 to +191
it("only one lock is created even with concurrent nextSessionRuleId calls", async () => {
const ids = await getSessionRuleIds();
expect(ids.size).toBeLessThan(100);

const added = [];
for (let w = ids.size; w < LIMIT_SESSION_RULES; w++) {
const j = await nextSessionRuleId();
added.push(j);
ids.add(j);
}
expect(ids.size).toBeGreaterThan(1000);

// Fire multiple concurrent calls while locked
const p1 = nextSessionRuleId();
const p2 = nextSessionRuleId();
const p3 = nextSessionRuleId();

const raceResult = await Promise.race([
Promise.all([p1, p2, p3]).then(() => "resolved"),
sleep(5).then(() => "pending"),
]);
expect(raceResult).toBe("pending");

// Single removal should unlock all waiters sequentially
const toRemove = [...ids].find((id) => id > 10000)!;
removeSessionRuleIdEntry(toRemove);

// All three should eventually resolve
const results = await Promise.race([Promise.all([p1, p2, p3]).then((ids) => ids), sleep(50).then(() => null)]);
expect(results).not.toBeNull();

Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

这两个并发相关用例期望“移除 1 个 id 后所有等待的 nextSessionRuleId() 都能继续并分配”,会掩盖/固化实际限额必须逐个放行的需求;在真实 DNR 配额下,这种行为可能重新触发 rule count exceeded。建议调整断言:每次 removeSessionRuleIdEntry 只应释放一个等待者(或显式验证不会超过 LIMIT_SESSION_RULES)。

Copilot uses AI. Check for mistakes.
Comment on lines 721 to 727
}
const redirectNotManual = params.redirect !== "manual";

// 使用 cacheInstance 避免SW重启造成重复 DNR Rule ID
const ruleId = 10000 + (await cacheInstance.incr("gmXhrRequestId", 1));
const ruleId = await nextSessionRuleId();
const rule = {
id: ruleId,
action: {
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

这里改为通过 nextSessionRuleId() 分配 ruleId 后,如果后续 updateSessionRules(addRules) 失败/抛错(例如仍触发 Session rule count exceeded),目前没有看到对应的回滚路径:headerModifierMap 与本地 ruleId 记录会残留,且 GM_xmlhttpRequest 的 catch 不会触发 loadendCleanUp,可能导致后续永久卡在限额锁。建议把 updateSessionRules 包在 try/catch 中,失败时立即清理 headerModifierMap 并释放该 ruleId(或延后 headerModifierMap.set 直到 updateSessionRules 成功)。

Copilot uses AI. Check for mistakes.
sessionRuleIdsPromise = chrome.declarativeNetRequest
.getSessionRules()
.then((rules) => {
sessionRuleIds = new Set(rules.map((rule) => rule.id).filter(Boolean));
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

getSessionRuleIds() 初始化时没有根据现有 session rule 的最大 id 更新 SESSION_RULE_ID_BEGIN,导致当历史遗留的 rule id 很大(旧逻辑是 cacheInstance.incr 单调递增)时,nextSessionRuleId() 可能从 10001 起做大量 do/while 递增扫描直到找到空洞,启动时会有明显 CPU 开销。建议在 rules 加载完成后把 SESSION_RULE_ID_BEGIN 设为 Math.max(SESSION_RULE_ID_BEGIN, maxExistingId),再在此基础上递增分配。

Suggested change
sessionRuleIds = new Set(rules.map((rule) => rule.id).filter(Boolean));
const existingRuleIds = rules.map((rule) => rule.id).filter(Boolean);
const maxExistingId = existingRuleIds.length > 0 ? Math.max(...existingRuleIds) : 0;
SESSION_RULE_ID_BEGIN = Math.max(SESSION_RULE_ID_BEGIN, maxExistingId);
sessionRuleIds = new Set(existingRuleIds);

Copilot uses AI. Check for mistakes.
Comment on lines +18 to +22
onResponseStarted = {
addListener: () => {
// TODO
},
};
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

WebRequest mock 的 onResponseStarted.addListener 目前是空实现,导致涉及该事件的逻辑(例如本次用于提前清理 DNR session rule)在单测里无法被触发/验证。建议与 onBeforeSendHeaders 一样缓存 callback(并提供触发入口),以便测试能够覆盖这条清理路径。

Copilot uses AI. Check for mistakes.
Comment on lines +133 to +146
const lockedPromise = nextSessionRuleId();

const raceResult1 = await Promise.race([lockedPromise.then(() => "resolved"), sleep(5).then(() => "pending")]);
expect(raceResult1).toBe("pending");

const m1 = Math.floor(Math.random() * (added.length - 9));
const p1 = added[m1];
const p2 = added[m1 + 6];
removeSessionRuleIdEntry(p1);
removeSessionRuleIdEntry(p2);

const raceResult2 = await Promise.race([lockedPromise.then(() => "resolved"), sleep(5).then(() => "pending")]);
expect(raceResult2).toBe("resolved");

Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

用 sleep(5)/sleep(50) 做并发锁的“是否 pending”判断在慢机器/CI 上可能波动导致偶发失败。建议使用更稳健的同步点(例如把 deferred 暴露出来、或用 fake timers/vi.useFakeTimers() 推进时间),避免依赖真实时间片。

Copilot uses AI. Check for mistakes.
Comment on lines +93 to +100
() => {
const lastError = chrome.runtime.lastError;
if (lastError) {
console.error("chrome.declarativeNetRequest.updateSessionRules:", lastError);
}
removeSessionRuleIdEntry(ruleID);
}
);
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

headersSettled() 在 updateSessionRules 回调里无论是否出现 lastError 都会调用 removeSessionRuleIdEntry(ruleID)。如果 removeRuleIds 失败(例如 DNR 状态异常/权限问题),本地会提前释放 ruleId,后续可能复用同一个 id 造成状态不一致,并且实际浏览器 session rules 数量也不会下降。建议仅在 lastError 为空时再 removeSessionRuleIdEntry;失败时保留本地记录并考虑重试/延迟清理,或至少不要复用该 id。

Copilot uses AI. Check for mistakes.
type ReceiveHeaderOptions = `${chrome.webRequest.OnHeadersReceivedOptions}` &
`${chrome.webRequest.OnResponseStartedOptions}`;

// 删除关联与DNR: 不再处理 headerModifer 时清空 Map 关联 及 浏览器 Session Rule
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

注释里的 "headerModifer" 拼写错误,建议改为 "headerModifier",避免后续搜索/维护时遗漏。

Suggested change
// 删除关联与DNR: 不再处理 headerModifer 时清空 Map 关联 及 浏览器 Session Rule
// 删除关联与DNR: 不再处理 headerModifier 时清空 Map 关联 及 浏览器 Session Rule

Copilot uses AI. Check for mistakes.
- dnr_id_controller: nextSessionRuleId 改为 while 循环反复判定上限,
  removeSessionRuleIdEntry 释放一次只放行一个 waiter,避免撑爆 LIMIT
- dnr_id_controller: 初始化时用历史最大 rule id 更新 SESSION_RULE_ID_BEGIN
- gm_api: headersSettled 在 lastError 时不释放 ruleID 避免本地/浏览器状态不一致
- gm_api: updateSessionRules addRules 失败时回滚 headerModifierMap 与 ruleId
- gm_api: 修正注释 headerModifer -> headerModifier
- chrome-extension-mock: onResponseStarted.addListener 暴露 callback 供测试触发
- dnr_id_controller.test: describe/it 标题改为中文,去除 Math.random
- dnr_id_controller.test: 并发用例改为 "1 次释放 = 1 个 waiter 放行" 语义
- vitest.setup: 移除注释掉的 spyOn 调试代码
@CodFrm
Copy link
Copy Markdown
Member

CodFrm commented Apr 22, 2026

test脚本有修改么?好像没带上来

@cyfung1031
Copy link
Copy Markdown
Collaborator Author

test脚本有修改么?好像没带上来

没有...
你按照刷课脚本 做一个?
反正我的环境不可能模仿出 5000个以上 xhr 并发

@CodFrm
Copy link
Copy Markdown
Member

CodFrm commented Apr 23, 2026

test脚本有修改么?好像没带上来

没有... 你按照刷课脚本 做一个? 反正我的环境不可能模仿出 5000个以上 xhr 并发

正常也不可能5000,电脑直接卡死了

@cyfung1031
Copy link
Copy Markdown
Collaborator Author

test脚本有修改么?好像没带上来

没有... 你按照刷课脚本 做一个? 反正我的环境不可能模仿出 5000个以上 xhr 并发

正常也不可能5000,电脑直接卡死了

你用虚拟机试一下 刷课脚本 吧..?
这个PR只是理论解决问题
我没有遇过这问题

@CodFrm CodFrm merged commit f8e76e6 into main Apr 23, 2026
4 checks passed
@CodFrm CodFrm deleted the pr/fix-multiple-xhr branch April 23, 2026 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P1 🔥 重要但是不紧急的内容

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] GM_xmlhttpRequest 高频请求时触发 Session rule count exceeded 报错,请求被拦截

3 participants