fix(sni-connect): add shared request admission queue - #81
Conversation
|
|
||
| do { | ||
| return try await SniConnectWallClockDeadline.run(timeoutMilliseconds: config.effectiveTotalTimeout) { | ||
| return try await SniConnectWallClockDeadline.run( |
There was a problem hiding this comment.
P1: [iOS deadline 竞态会在超时后返回成功]
当 executor 因高负载或 app 恢复而延迟调度时，operation 与 timeout child 可能都在 deadline 之后才变为可运行。这里调用的 SniConnectWallClockDeadline.run 只按 child 完成顺序取结果，没有在返回 operation 结果前重新核对 deadline，因此已超时的 transport 仍可能成功 resolve；HEAD 上完整 Swift suite 已连续两次触发 testWallClockDeadlineTimesOutSlowOperation 失败。
请在 SniConnectWallClockDeadline 中用 ContinuousClock 保存绝对 deadline，并在返回 operation 结果前再次检查，过期时统一抛出 deadlineExceeded。
| maxRequests = SniConnectValidation.MAX_ACTIVE_REQUESTS | ||
| maxRequestsPerHost = SniConnectValidation.MAX_ACTIVE_REQUESTS | ||
| } | ||
| private val sharedAdmission = SniConnectRequestAdmission() |
There was a problem hiding this comment.
P2: [runtime 销毁后共享准入槽位不会立即释放]
当 main/background RN runtime 在请求 pending 或 active 时被销毁，目前没有 invalidate/onCatalystInstanceDestroy 清理该实例的 allActiveCalls。sharedAdmission 会通过 ticket callback 继续持有这些请求，直到 transport 完成或最长 120 秒 deadline，期间孤儿请求仍占用进程级 64 active/256 pending 配额并可能阻塞新 runtime；iOS 的 shared limiter 也有相同生命周期缺口。
请为 Android NativeModule 和 iOS RCT module 增加对称的 invalidation 路径，在 teardown 时取消该实例拥有的全部请求并释放对应 ticket/token。
| val requestId = "req-${index.toString().padStart(2, '0')}" | ||
| admission.createTicket( | ||
| hostname = "Example.com", | ||
| ip = if (index % 2 == 0) "093.184.216.034" else "93.184.216.34", |
There was a problem hiding this comment.
P2: [零填充 IPv4 在 Android 与 iOS 上行为不一致]
这里把 093.184.216.034 与 93.184.216.34 视为等价合法输入：Kotlin 的正则和 toInt() 会接受前者，但 iOS 的 inet_pton 会将其拒绝为 invalidIP。因此同一 JS 请求会在 Android 成功、在 iOS 返回 SNI_INVALID_CONFIG，而零填充 octet 本身也不符合标准 IPv4 dec-octet 文法。
请收紧 Android IPv4 parser，除单独的 0 外拒绝带前导零的 octet，并改用双平台都接受的等价 IPv6 表示测试 canonical pair key。
OneKey Auto-review ·
|
| 字段 | 结果 |
|---|---|
| 结论 | FIX_AND_RERUN |
| Risk / Confidence | high / high |
| Profile | standard |
| Reviewed SHA | c4f7864d064ce41e78c8890dc3144be9acdee2cb |
| Inline 发布 | accepted · selected 3, posted 3, skipped 0, failed 0 |
| Findings 去向 | confirmed inline 3 · report-only 0 |
Review 摘要
该 PR 将 react-native-sni-connect 的请求资源所有权从各 RN runtime 内的即时限流，提升为 iOS、Android 进程共享的有界准入状态机：全局最多 64 个 active、每个 canonical (hostname, ip) 最多 16 个 active、最多 256 个 cancellable pending；排队时间计入总 deadline，transport 仅接收剩余 timeout。变更同时贯通 request ID 取消、原生 debug snapshot、双平台测试和规范文档，并按 workspace 发布约定将 35 个 package 升级至 3.0.81。
人工关注点： 合入前应优先修复当前 HEAD 可复现的 iOS deadline 竞态，然后补齐 RN runtime teardown 对共享槽位的清理，并让 iOS process-shared admission 与 process-shared resolver registry 使用一致的资源所有权。
Findings
- P1 · iOS deadline 竞态会在超时后返回成功
- 影响：当 Swift executor 因负载、app 恢复或调度延迟而让 operation 与 timeout child 都在 deadline 后才变为可运行时，
SniConnectWallClockDeadline.run只消费先被调度完成的 child，没有在返回 operation 结果前重新核对绝对 deadline。该 PR 继续用它执行剩余 transport budget，因此请求可能在总 timeout 后仍成功 resolve。当前 HEAD 的完整 Swift suite 已实际出现testWallClockDeadlineTimesOutSlowOperation返回late而非抛出 timeout，20 个测试中有 1 个失败。 - 建议：用
ContinuousClock为请求保存单调的绝对 deadline，并在 deadline helper 返回 operation 结果前再次核对是否过期；随后重复运行完整 Swift suite 和调度延迟场景。 - Inline 决策：
yes
- 影响：当 Swift executor 因负载、app 恢复或调度延迟而让 operation 与 timeout child 都在 deadline 后才变为可运行时，
- P2 · runtime 销毁后共享准入槽位不会立即释放
- 影响：当 main/background RN runtime 在请求 pending 或 active 时被销毁，Android 没有通过
invalidate/onCatalystInstanceDestroy清理该实例的allActiveCalls，iOS 也没有从 RCT invalidation 转发到cancelAllRequests。静态 admission/limiter 会继续通过 callback、continuation 或 Task 持有这些请求，直到 transport 完成或最长 120 秒 deadline；期间孤儿 work 仍占用进程级 64 active/256 pending 配额并可能阻塞后来创建的 runtime。 - 建议：为 Android NativeModule 和 iOS RCT module 增加对称的 lifecycle invalidation，在 teardown 时原子快照并取消该实例拥有的全部请求，使 pending ticket 和 active token 立即走现有释放路径。
- Inline 决策：
yes
- 影响：当 main/background RN runtime 在请求 pending 或 active 时被销毁，Android 没有通过
- P2 · iOS resolver registry 未纳入进程共享准入
- 影响：iOS 的
SniConnectRequestLimiter.shared已按进程准入，但每个SniConnectClient仍拥有独立 session cache，而PinnedDNSResolverFactory.registry是全进程共享且最多 32 项。一个 runtime 缓存 32 个不同 pair 后，另一个 runtime 对第 33 个新 pair 会先成功取得 admission token，随后因自己的空 cache 无法淘汰前一 runtime 的 idle resolver 而直接抛出SNI_RESOURCE_LIMIT。此时 pending queue 可以完全未溢出，违反本 PR 新增的“仅 pending overflow 才拒绝”协议，并形成 iOS 特有的跨 runtime 失败。 - 建议：将 iOS resolver slot 和 session cache 统一交给进程共享的 owner，并在该全局边界内跨 runtime 淘汰 idle entry或等待 slot，再启动 transport。
- Inline 决策：
yes
- 影响：iOS 的
Validation Gaps
完整报告未单独列出 Validation Gaps；请展开下方报告核对验证范围。
完整 review 报告
# PR #81 Code ReviewScope
- Review Mode: PR
- PR URL: fix(sni-connect): add shared request admission queue #81
- Baseline:
origin/main...HEAD - HEAD:
c4f7864d064ce41e78c8890dc3144be9acdee2cb - Merge Base:
ef1492509e2f0105c13ad93dcfefaa3ea00925d3 - Change Size: 51 files，+1411 / -264
- Platforms: iOS、Android、TypeScript
- Modules:
react-native-sni-connect；另有 35 个 workspace package version bump - Risk: High
- Codex Cross-validation: 未启用
PR Overview
该 PR 首先改变 SNI 请求的系统所有权边界：iOS、Android 不再由单个 RN runtime 在达到 active 上限时即时拒绝，而是通过进程共享的准入状态机管理 created → pending → active → terminal，执行全局 64、单 canonical (hostname, ip) 16、pending 256 的有界策略。排队请求支持 request ID 取消，总 deadline 覆盖 queue wait，transport 只接收剩余 timeout。
公开行为方面，TypeScript、TurboModule/legacy bridge 与双平台原生实现新增 getDebugSnapshot，用于读取进程级计数及目标 pair 的 request IDs；文档和 package version 同步更新。可维护性和可读性方面，Android 将 admission 拆为独立类，iOS 将 limiter 扩展为 continuation 驱动的异步队列，测试覆盖容量、canonical key、取消、超时和恢复。安全方面，有界 queue 改善了资源耗尽防护，既有 TLS、public-IP、header 和 proxy bypass 边界未被削弱；但 deadline、runtime lifecycle 和 iOS resolver ownership 仍未完整闭合。
Overengineering Assessment
- Verdict:
not_present - Rationale: 双平台状态机、取消路径、debug snapshot、规范和测试均直接对应进程共享准入目标，没有无关功能、推测性扩展点或重复基础设施。发现的问题属于必要所有权边界未完整实现，而非方案范围过大。
- Evidence:
- Kotlin admission 类直接承载 64/16/256 状态边界及 race-safe cancellation。
- Swift limiter 在既有组件上扩展 pending queue，没有引入第二套调度基础设施。
getDebugSnapshot沿 TypeScript、双架构 bridge 和原生 limiter 完整贯通并有测试消费。- workspace-wide patch version bump 属于仓库既有发布边界。
Scorecard
| Dimension | Score | Notes |
|---|---|---|
| Security | 8.0/10 | 有界队列增强资源防护，未发现 TLS 或输入校验退化。 |
| Correctness | 4.5/10 | iOS 总 deadline 竞态已由当前完整测试复现。 |
| Architecture | 5.5/10 | process-shared admission 合理，但 teardown 与 resolver ownership 未对齐。 |
| Maintainability | 6.5/10 | 状态机和单元测试较清晰，跨 runtime 生命周期协议仍不完整。 |
| Completeness | 5.0/10 | Swift suite 未通过，且缺少双 runtime teardown/resolver 集成验证。 |
| Total | 6.1/10 | 需修改后复审。 |
Detailed Findings
F-001 — P1: iOS deadline 竞态会在超时后返回成功
- Path:
native-modules/react-native-sni-connect/ios/SniConnectClient.swift:711 - Confidence: high
- Type: Correctness
当 executor 延迟调度，使 operation 与 timeout child 都在 deadline 后进入可运行状态时，SniConnectWallClockDeadline.run 只按 child 完成顺序选择结果，没有在返回 operation 前检查绝对 deadline。该 PR 将剩余 transport budget 继续交给此 helper，因此超时请求仍可能成功 resolve。
当前 HEAD 使用 Xcode toolchain 执行完整 Swift suite，20 个测试中有 1 个失败：testWallClockDeadlineTimesOutSlowOperation 返回了 late。应以 ContinuousClock 保存绝对 deadline，并在返回 operation 结果前再次核对是否过期。
F-002 — P2: runtime 销毁后共享准入槽位不会立即释放
- Path:
native-modules/react-native-sni-connect/android/src/main/java/com/sniconnect/SniConnectModule.kt:98 - Related Path:
native-modules/react-native-sni-connect/ios/SniConnectClient.swift:123 - Confidence: high
- Type: Architecture
准入器改为 process-shared 后，runtime teardown 必须释放该 runtime 拥有的 work。当前 Android 没有 native module invalidation 清理，iOS 也没有对应的 RCT invalidation 转发；pending callback、continuation 和 active Task 会继续持有实例及全局槽位，最长持续到 120 秒 timeout。
应在 Android/iOS lifecycle callback 中原子取消本实例全部请求，并复用现有 cancellation path 释放 ticket/token。
F-003 — P2: iOS resolver registry 未纳入进程共享准入
- Path:
native-modules/react-native-sni-connect/ios/SniConnectClient.swift:123 - Related Path:
native-modules/react-native-sni-connect/ios/SniConnectCore.swift:51 - Confidence: high
- Type: Architecture / Runtime
limiter 已是全局 singleton，但 session cache 仍属于每个 client，底层 resolver registry 则全进程最多 32 项。runtime A 缓存 32 个 pair 后，runtime B 的新 pair 无法从自己的空 cache 淘汰 A 的 idle session，因而在 admission queue 未溢出时直接抛出 SNI_RESOURCE_LIMIT。
应将 resolver slot 与 session cache 纳入同一个 process-shared owner，在全局边界内协调 idle eviction 或 slot wait。
Validation Gaps
git diff --check origin/main...HEAD通过；最终 worktree clean，HEAD 与 merge base 保持不变。- 使用 Xcode toolchain 构建 Swift validation core 成功；完整
swift test --disable-sandbox执行 20 个测试时有 1 个失败：testWallClockDeadlineTimesOutSlowOperation。 - SwiftPM target 排除了依赖 React、UIKit、EMASCurl 的完整 iOS bridge/client，仍需实际 iOS 工程构建验证。
- Android Gradle unit tests 未完成：默认 Gradle home 在受管环境中不可写，且可写临时目录没有缓存 wrapper distribution。
- Jest 与 TypeScript typecheck 未完成：当前 workspace 缺少 Yarn
node_modulesstate。 - 尚无 main/background RN runtime teardown、跨 runtime resolver saturation 或受控 HTTPS transport integration test。
- 当前执行环境无法连接 GitHub API，因此现有 PR comments 未纳入复核。
Recommendation
先修复 P1 deadline 竞态，再补齐双平台 runtime invalidation 和 iOS process-shared resolver/session ownership。随后重复运行完整 Swift suite，并在具备依赖的环境执行 Android unit tests、Jest、typecheck、完整 iOS/Android build 及双 runtime 集成验证；通过后再复审。
|
|
||
| do { | ||
| return try await SniConnectWallClockDeadline.run(timeoutMilliseconds: config.effectiveTotalTimeout) { | ||
| return try await SniConnectWallClockDeadline.run( |
There was a problem hiding this comment.
P1: [iOS deadline 竞态会在超时后返回成功]
当 Swift executor 因高负载或 app 恢复而延迟调度时，operation 与 timeout child 可能都在 deadline 后才变为可运行。这里调用的 SniConnectWallClockDeadline.run 只按 child 完成顺序取结果，没有在返回 operation 结果前重新核对绝对 deadline，因此已经超时的 transport 仍可能成功 resolve；当前 HEAD 的完整 Swift suite 也实际出现了 testWallClockDeadlineTimesOutSlowOperation 返回 late 的失败。
请在 deadline helper 中用 ContinuousClock 保存绝对 deadline，并在返回 operation 结果前再次检查，过期时统一抛出 deadlineExceeded。
| maxRequests = SniConnectValidation.MAX_ACTIVE_REQUESTS | ||
| maxRequestsPerHost = SniConnectValidation.MAX_ACTIVE_REQUESTS | ||
| } | ||
| private val sharedAdmission = SniConnectRequestAdmission() |
There was a problem hiding this comment.
P2: [runtime 销毁后共享准入槽位不会立即释放]
当 main/background RN runtime 在请求 pending 或 active 时被销毁，目前 Android 没有通过 invalidate/onCatalystInstanceDestroy 取消该实例的 allActiveCalls，iOS 也没有对应的 RCT invalidation 转发。静态 admission/limiter 会继续持有这些孤儿请求，直到 transport 完成或最长 120 秒 deadline，期间它们仍占用进程级 64 active/256 pending 配额并可能阻塞新 runtime。
请为 Android NativeModule 和 iOS RCT module 增加对称的 lifecycle invalidation，在 teardown 时原子取消该实例拥有的全部请求，让现有 cancellation path 立即释放 ticket/token。
| private var requestTokensById: [String: UUID] = [:] | ||
| private let tasksQueue = DispatchQueue(label: "com.onekey.sni.connect.tasks", attributes: .concurrent) | ||
| private let requestLimiter = SniConnectRequestLimiter() | ||
| private let requestLimiter = SniConnectRequestLimiter.shared |
There was a problem hiding this comment.
P2: [iOS resolver registry 未纳入进程共享准入]
当一个 RN runtime 已缓存 32 个不同 (hostname, ip) session 后，另一个 runtime 请求新 pair 时，这个 shared limiter 仍会正常授予 active token；但每个 client 的 session cache 彼此独立，而 PinnedDNSResolverFactory.registry 是全进程共享且最多 32 项，第二个 runtime 无法淘汰第一个 runtime 的 idle resolver，最终会在 pending queue 未溢出时直接收到 SNI_RESOURCE_LIMIT。
请把 iOS resolver slot 和 session cache 统一交给进程共享的 owner，在同一全局边界内协调跨 runtime 的 idle eviction 或 slot wait，再启动 transport。
2 participants
Summary
requestId, per-runtime cancellation APIs, and debug snapshots to the public SNI JS/native API3.0.81and document the release inCHANGELOG.mdWhy
The app-monorepo currently carries these SNI request-queue and cancellation behaviors as a package patch. Moving them into the maintained package source removes that patch dependency and ensures main/background React Native runtimes share capacity while retaining cancellation ownership per runtime.
Impact
@onekeyfe/react-native-sni-connect@3.0.81and remove the downstream patchValidation
corepack yarn install --immutable? passedcorepack yarn workspace @onekeyfe/react-native-sni-connect prepare? passedcorepack yarn workspace @onekeyfe/react-native-sni-connect typecheck? passedcorepack yarn workspace @onekeyfe/react-native-sni-connect test --runInBand? passed (6/6)corepack yarn workspace @onekeyfe/react-native-sni-connect test:android? passed (19/19)swift test? passed (20/20)BUILD SUCCEEDED)RCT_NEW_ARCH_ENABLED=0git diff --check? passedThe iOS runtime harness does not cover real-network
cancelAllRequests, response-body cancellation, cross-runtime ownership, or duplicate-ID replacement; those paths were source-reviewed and compiled under New Architecture.Publishing
@onekeyfe/react-native-sni-connect@3.0.81.3.0.81.3.0.81was rejected as a duplicate; its log contains 34 successful publishes and no other npm error.latest=3.0.81.