Skip to content

Commit 276a640

Browse files
committed
处理 code review 反馈: 修复并发锁/错误回滚并改善测试
- 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 调试代码
1 parent 98cc9bb commit 276a640

5 files changed

Lines changed: 78 additions & 44 deletions

File tree

packages/chrome-extension-mock/web_reqeuest.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import EventEmitter from "eventemitter3";
22

33
export default class WebRequest {
44
sendHeader?: (details: chrome.webRequest.OnSendHeadersDetails) => chrome.webRequest.BlockingResponse | void;
5+
responseStarted?: (details: chrome.webRequest.OnResponseStartedDetails) => void;
56

67
onBeforeSendHeaders = {
78
addListener: (callback: any) => {
@@ -16,8 +17,8 @@ export default class WebRequest {
1617
};
1718

1819
onResponseStarted = {
19-
addListener: () => {
20-
// TODO
20+
addListener: (callback: any) => {
21+
this.responseStarted = callback;
2122
},
2223
};
2324

src/app/service/service_worker/gm_api/dnr_id_controller.test.ts

Lines changed: 45 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
} from "./dnr_id_controller";
99

1010
describe("getSessionRuleIds", () => {
11-
it("initializes from existing chrome session rules", async () => {
11+
it("从现有 chrome session rules 初始化", async () => {
1212
const ids = await getSessionRuleIds();
1313
expect(ids.size).lessThan(100);
1414
await nextSessionRuleId();
@@ -19,7 +19,7 @@ describe("getSessionRuleIds", () => {
1919
});
2020

2121
describe("nextSessionRuleId", () => {
22-
it("returns unique incrementing IDs on each call", async () => {
22+
it("每次调用返回唯一递增的 id", async () => {
2323
const id1 = await nextSessionRuleId();
2424
const id2 = await nextSessionRuleId();
2525
const id3 = await nextSessionRuleId();
@@ -28,7 +28,7 @@ describe("nextSessionRuleId", () => {
2828
expect(id3).toBeGreaterThan(id2);
2929
});
3030

31-
it("skips IDs that already exist in session rules", async () => {
31+
it("跳过已存在于 session rules 中的 id", async () => {
3232
const ids = await getSessionRuleIds();
3333
const next = await nextSessionRuleId();
3434

@@ -40,7 +40,7 @@ describe("nextSessionRuleId", () => {
4040
});
4141

4242
describe("removeSessionRuleIdEntry", () => {
43-
it("removes the given ID from the tracked set", async () => {
43+
it("从追踪集合中移除指定 id", async () => {
4444
const ids = await getSessionRuleIds();
4545
const id = await nextSessionRuleId();
4646

@@ -50,16 +50,16 @@ describe("removeSessionRuleIdEntry", () => {
5050
expect(ids.has(id)).toBe(false);
5151
});
5252

53-
it("is a no-op when called before sessionRuleIds is initialized", () => {
53+
it("sessionRuleIds 未初始化时为 no-op", () => {
5454
expect(() => removeSessionRuleIdEntry(10404)).not.toThrow();
5555
});
5656

57-
it("throws when ruleId is <= 10000", () => {
57+
it("ruleId <= 10000 时抛错", () => {
5858
expect(() => removeSessionRuleIdEntry(10000)).toThrow();
5959
expect(() => removeSessionRuleIdEntry(1)).toThrow();
6060
});
6161

62-
it("rewinds SESSION_RULE_ID_BEGIN so the removed id gets reused next", async () => {
62+
it("回退 SESSION_RULE_ID_BEGIN 使被移除的 id 下次被复用", async () => {
6363
const ids = await getSessionRuleIds();
6464

6565
const id1 = await nextSessionRuleId();
@@ -70,13 +70,13 @@ describe("removeSessionRuleIdEntry", () => {
7070
ids.add(id2);
7171
ids.add(id3);
7272

73-
// Remove the latest — counter should rewind and reuse it
73+
// 移除最新的 id,counter 回退并复用该 id
7474
removeSessionRuleIdEntry(id3);
7575
const reused = await nextSessionRuleId();
7676
expect(reused).toBe(id3);
7777
});
7878

79-
it("rewinds SESSION_RULE_ID_BEGIN when removed id is behind the counter", async () => {
79+
it("移除的 id counter 之前时仍会回退", async () => {
8080
const ids = await getSessionRuleIds();
8181

8282
const id1 = await nextSessionRuleId();
@@ -87,13 +87,13 @@ describe("removeSessionRuleIdEntry", () => {
8787
ids.add(id2);
8888
ids.add(id3);
8989

90-
// Remove an older id — counter rewinds to id1 - 1, so id1 gets reused
90+
// 移除较早的 id,counter 回退到 id1 - 1,下次分配会得到 id1
9191
removeSessionRuleIdEntry(id1);
9292
const reused = await nextSessionRuleId();
9393
expect(reused).toBe(id1);
9494
});
9595

96-
it("does not rewind SESSION_RULE_ID_BEGIN when removed id was pre-existing (ahead of counter)", async () => {
96+
it("移除 counter 之后的 id 不回退 counter", async () => {
9797
const id1 = await nextSessionRuleId();
9898
const id2 = await nextSessionRuleId();
9999
const id3 = await nextSessionRuleId();
@@ -109,16 +109,16 @@ describe("removeSessionRuleIdEntry", () => {
109109
const nextBefore = await nextSessionRuleId(); // e.g. 10001
110110
removeSessionRuleIdEntry(id6);
111111

112-
expect(await nextSessionRuleId()).toBe(nextBefore + 1); // counter unchanged, just increments normally
113-
expect(await nextSessionRuleId()).toBe(nextBefore + 2); // counter unchanged, just increments normally
114-
// expect(await nextSessionRuleId()).toBe(nextBefore + 3); // counter unchanged, just increments normally
115-
expect(await nextSessionRuleId()).toBe(nextBefore + 4); // counter unchanged, just increments normally
116-
expect(await nextSessionRuleId()).toBe(nextBefore + 5); // counter unchanged, just increments normally
112+
expect(await nextSessionRuleId()).toBe(nextBefore + 1); // counter 未变,正常递增
113+
expect(await nextSessionRuleId()).toBe(nextBefore + 2);
114+
// expect(await nextSessionRuleId()).toBe(nextBefore + 3);
115+
expect(await nextSessionRuleId()).toBe(nextBefore + 4);
116+
expect(await nextSessionRuleId()).toBe(nextBefore + 5);
117117
});
118118
});
119119

120120
describe("nextSessionRuleId limit control", () => {
121-
it("locks when session rules reach the limit and unlocks when an entry is removed", async () => {
121+
it("达到上限时锁定,移除条目后解锁", async () => {
122122
const ids = await getSessionRuleIds();
123123
expect(ids.size).toBeLessThan(100);
124124

@@ -135,9 +135,9 @@ describe("nextSessionRuleId limit control", () => {
135135
const raceResult1 = await Promise.race([lockedPromise.then(() => "resolved"), sleep(5).then(() => "pending")]);
136136
expect(raceResult1).toBe("pending");
137137

138-
const m1 = Math.floor(Math.random() * (added.length - 9));
139-
const p1 = added[m1];
140-
const p2 = added[m1 + 6];
138+
// 使用固定索引而非随机,保证测试可重复
139+
const p1 = added[0];
140+
const p2 = added[6];
141141
removeSessionRuleIdEntry(p1);
142142
removeSessionRuleIdEntry(p2);
143143

@@ -158,7 +158,7 @@ describe("nextSessionRuleId limit control", () => {
158158
expect(res.size).toBeLessThan(100);
159159
});
160160

161-
it("only one lock is created even with concurrent nextSessionRuleId calls", async () => {
161+
it("单次移除仅放行 1 个 waiter,其余继续等待", async () => {
162162
const ids = await getSessionRuleIds();
163163
expect(ids.size).toBeLessThan(100);
164164

@@ -170,7 +170,7 @@ describe("nextSessionRuleId limit control", () => {
170170
}
171171
expect(ids.size).toBeGreaterThan(1000);
172172

173-
// Fire multiple concurrent calls while locked
173+
// 在已达上限时发起多个并发调用
174174
const p1 = nextSessionRuleId();
175175
const p2 = nextSessionRuleId();
176176
const p3 = nextSessionRuleId();
@@ -181,16 +181,33 @@ describe("nextSessionRuleId limit control", () => {
181181
]);
182182
expect(raceResult).toBe("pending");
183183

184-
// Single removal should unlock all waiters sequentially
185-
const toRemove = [...ids].find((id) => id > 10000)!;
186-
removeSessionRuleIdEntry(toRemove);
184+
// 单次释放 1 个 slot: 只应放行 1 个 waiter,剩余仍挂起
185+
removeSessionRuleIdEntry(added[0]);
186+
const firstResolved = await Promise.race([p1.then(() => "resolved"), sleep(50).then(() => "pending")]);
187+
expect(firstResolved).toBe("resolved");
187188

188-
// All three should eventually resolve
189-
const results = await Promise.race([Promise.all([p1, p2, p3]).then((ids) => ids), sleep(50).then(() => null)]);
189+
const remainingStillPending = await Promise.race([
190+
Promise.all([p2, p3]).then(() => "resolved"),
191+
sleep(5).then(() => "pending"),
192+
]);
193+
expect(remainingStillPending).toBe("pending");
194+
195+
// 继续释放 2 个 slot,剩余 waiter 才能继续完成
196+
removeSessionRuleIdEntry(added[1]);
197+
removeSessionRuleIdEntry(added[2]);
198+
const results = await Promise.race([Promise.all([p2, p3]).then((vals) => vals), sleep(50).then(() => null)]);
190199
expect(results).not.toBeNull();
191200

201+
// 任何时候 size 都不应超过 LIMIT_SESSION_RULES
202+
expect(ids.size).toBeLessThanOrEqual(LIMIT_SESSION_RULES);
203+
192204
for (const k of added) {
193-
removeSessionRuleIdEntry(k);
205+
if (ids.has(k)) removeSessionRuleIdEntry(k);
206+
}
207+
// p1/p2/p3 分配的 id 也要清理
208+
const allocated = [await p1, ...(results as number[])];
209+
for (const k of allocated) {
210+
if (ids.has(k)) removeSessionRuleIdEntry(k);
194211
}
195212

196213
const res = await getSessionRuleIds();

src/app/service/service_worker/gm_api/dnr_id_controller.ts

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,13 @@ export const getSessionRuleIds = async (): Promise<Set<number>> => {
1414
sessionRuleIdsPromise = chrome.declarativeNetRequest
1515
.getSessionRules()
1616
.then((rules) => {
17-
sessionRuleIds = new Set(rules.map((rule) => rule.id).filter(Boolean));
17+
const existingRuleIds = rules.map((rule) => rule.id).filter(Boolean);
18+
// 根据历史 session rule 的最大 id 更新 SESSION_RULE_ID_BEGIN
19+
// 避免 SW 重启后从 10001 起做大量 do/while 递增扫描
20+
if (existingRuleIds.length > 0) {
21+
SESSION_RULE_ID_BEGIN = Math.max(SESSION_RULE_ID_BEGIN, ...existingRuleIds);
22+
}
23+
sessionRuleIds = new Set(existingRuleIds);
1824
return sessionRuleIds;
1925
})
2026
.catch((e) => {
@@ -36,18 +42,21 @@ export const removeSessionRuleIdEntry = (ruleId: number) => {
3642
if (ruleId <= SESSION_RULE_ID_BEGIN + 1) {
3743
SESSION_RULE_ID_BEGIN = ruleId - 1;
3844
}
39-
if (sessionRuleIds.size < LIMIT_SESSION_RULES) {
40-
lockSessionRuleCreation?.resolve();
41-
lockSessionRuleCreation = null;
42-
}
45+
// 唤醒所有等待者: 每个等待者会在 while 循环内重新判定 size 并决定是继续分配还是再次等待
46+
// 这样 "1 次释放 => 1 个等待者通过",避免 N 个等待者同时放行再次撑爆上限
47+
lockSessionRuleCreation?.resolve();
48+
lockSessionRuleCreation = null;
4349
}
4450
}
4551
};
4652

4753
export const nextSessionRuleId = async () => {
4854
const ruleIds = await getSessionRuleIds();
49-
if (!lockSessionRuleCreation && ruleIds.size + 1 > LIMIT_SESSION_RULES) lockSessionRuleCreation = deferred<void>();
50-
if (lockSessionRuleCreation) await lockSessionRuleCreation.promise;
55+
// 用 while 循环反复判定上限: 等待者被唤醒后如果 slot 已被其他等待者抢占,会再次进入等待
56+
while (ruleIds.size + 1 > LIMIT_SESSION_RULES) {
57+
if (!lockSessionRuleCreation) lockSessionRuleCreation = deferred<void>();
58+
await lockSessionRuleCreation.promise;
59+
}
5160
let id;
5261
do {
5362
id = ++SESSION_RULE_ID_BEGIN;

src/app/service/service_worker/gm_api/gm_api.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ type OnBeforeSendHeadersOptions = `${chrome.webRequest.OnBeforeSendHeadersOption
7878
type ReceiveHeaderOptions = `${chrome.webRequest.OnHeadersReceivedOptions}` &
7979
`${chrome.webRequest.OnResponseStartedOptions}`;
8080

81-
// 删除关联与DNR: 不再处理 headerModifer 时清空 Map 关联 及 浏览器 Session Rule
81+
// 删除关联与DNR: 不再处理 headerModifier 时清空 Map 关联 及 浏览器 Session Rule
8282
const headersSettled = (markerID: string) => {
8383
const dnrRule = headerModifierMap.get(markerID);
8484
const ruleID = dnrRule?.rule.id;
@@ -93,7 +93,9 @@ const headersSettled = (markerID: string) => {
9393
() => {
9494
const lastError = chrome.runtime.lastError;
9595
if (lastError) {
96+
// removeRuleIds 失败: 浏览器里仍保留该规则,本地不释放 ruleID 避免复用
9697
console.error("chrome.declarativeNetRequest.updateSessionRules:", lastError);
98+
return;
9799
}
98100
removeSessionRuleIdEntry(ruleID);
99101
}
@@ -738,10 +740,17 @@ export default class GMApi {
738740
},
739741
} as chrome.declarativeNetRequest.Rule;
740742
headerModifierMap.set(markerID, { rule, redirectNotManual });
741-
await chrome.declarativeNetRequest.updateSessionRules({
742-
removeRuleIds: [ruleId],
743-
addRules: [rule],
744-
});
743+
try {
744+
await chrome.declarativeNetRequest.updateSessionRules({
745+
removeRuleIds: [ruleId],
746+
addRules: [rule],
747+
});
748+
} catch (e) {
749+
// addRules 失败: 回滚本地 headerModifierMap 关联并释放 ruleId,避免永久占位导致限额锁死
750+
headerModifierMap.delete(markerID);
751+
removeSessionRuleIdEntry(ruleId);
752+
throw e;
753+
}
745754
}
746755
return true;
747756
}

tests/vitest.setup.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ import { mockFetch } from "./mocks/fetch";
99

1010
vi.stubGlobal("chrome", chromeMock);
1111
chromeMock.init();
12-
// vi.spyOn(chromeMock.declarativeNetRequest, "getSessionRules");
13-
// vi.spyOn(chromeMock.declarativeNetRequest, "updateSessionRules");
1412
initTestEnv();
1513

1614
chromeMock.runtime.getURL = vi.fn().mockImplementation((path: string) => {

0 commit comments

Comments
 (0)