`getImport()` returns the id of an already-settled import, sending a released id to the peer

已关闭 适合新手
#265 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

维护者通常 5 天内回复

已经有一个关联 PR 被合并了。

  • #267 来自 @Kanishkrawatt —— 已合并

评估

难度
2/5
预计耗时
1-3 小时
新手友好度
76/100
Issue 类型
缺陷
描述清晰度
描述清楚
活跃度
活跃

调研方向

从 dist/index.js 中 2036 行附近的 getImport() 开始,然后比较 call、stream、map、get 和 pull 中的解析检查。检查 1329-1330 行附近的 serializer,并运行 issue 中的复现步骤。当传入一个已 settled 的 promise 不会发出其已释放的 import id,且 session 仍可用时,即表示完成。

由索引模型根据 Issue 内容生成。

描述

Summary

getImport() returns an import's id without checking whether that import has already settled and been released. Passing a settled RpcPromise as an argument therefore puts a dead id on the wire, and the peer aborts the entire session.

Reproduced on 0.12.0 (also on 0.10.0) with both peers on identical code, so version skew is not required.

Repro
import { RpcSession, RpcTarget } from 'capnweb'

// in-memory transport pair, logging every frame
const mk = (label) => { const q = []; let w = null
  return { push: (m) => { if (w) { const r = w; w = null; r(m) } else q.push(m) },
    t: { send(m) { console.log(`${label} ->`, m); peer[label === 'client' ? 'server' : 'client'].push(m) },
         receive() { return q.length ? Promise.resolve(q.shift()) : new Promise(r => { w = r }) },
         abort(e) { console.log(`${label} transport abort:`, String(e)) } } }
}
const c = mk('client'), s = mk('server'); const peer = { client: c, server: s }

class Api extends RpcTarget {
  getThing() { return { n: 1 } }
  use(x)     { return { echoed: x } }
}
const server = new RpcSession(s.t, new Api())
const client = new RpcSession(c.t)
const api = client.getRemoteMain()

const p = api.getThing()
const v = await p

await api.use(v)   // fine
await api.use(p)   // passes the same promise object, not its value
await api.getThing()   // session is gone
Actual output
--- await, then pass the VALUE ---
client -> ["push",["pipeline",0,["getThing"],[]]]
client -> ["pull",1]
server -> ["resolve",1,{"n":1}]
client -> ["release",1,1]
v = { n: 1 }
client -> ["push",["pipeline",0,["use"],[{"n":1}]]]
client -> ["pull",2]
server -> ["resolve",2,{"echoed":{"n":1}}]
client -> ["release",2,1]
use(v) = { echoed: { n: 1 } }

--- await, then pass the same PROMISE object ---
client -> ["push",["pipeline",0,["use"],[["pipeline",1]]]]
client -> ["pull",3]
server -> ["abort",["error","Error","no such entry on exports table: 1"]]
server transport abort: Error: no such entry on exports table: 1
client transport abort: Error: no such entry on exports table: 1
use(p) THREW: no such entry on exports table: 1

--- is the session still usable? ---
SESSION DEAD: no such entry on exports table: 1

The use(p) frame is the bug: ["pipeline",1] names import 1, which the client itself released two frames earlier with ["release",1,1].

Expected

Passing a settled RpcPromise as an argument should either resolve locally from the value already held, or fail that one call. It should not end the session. Note that passing the awaited value (use(v)) has always worked; passing the promise object (use(p)) is the trigger, and the two are easy to confuse in application code.

Cause

This looks like a missed case in an established pattern rather than a missing pattern.

ImportTableEntry.resolve() stores the value in this.resolution and immediately calls this.sendRelease() (dist/index.js:1750, :1756), which sends the release frame and zeroes remoteRefcount (:1806). Both sides then correctly agree the id is dead. The id itself has to survive settlement, because sendRelease() reads this.importId to name what it releases. What should not survive is its use.

Five accessors already know this and operate on the settled value instead of the wire:

accessor line (0.12.0 dist/index.js)
call :1839
stream :1844
map :1851
get :1856
pull :1865

getImport() (:2036) does not. It asks only whether the hook belongs to this session:

getImport(hook) {
  if (hook instanceof RpcImportHook && hook.entry && hook.entry.session === this) return hook.entry.importId
  else return
}

The resulting frame is fatal on the peer because evaluateImpl throws no such entry on exports table (:1586, :1618, :1628) inside readLoop, whose only error handler is session-wide: this.readLoop().catch((err) => this.abort(err)) (:1929). That second half is #266, filed separately since it is a distinct decision; this issue is about not emitting the bad frame in the first place.

Suggested fix

Extend the same entry.resolution gate to getImport(), so a settled entry withholds its id and the caller's existing undefined path takes over (the serializer calls it at :1329 and already branches on importId !== void 0 at :1330, which is the site that emits the ["pipeline", importId, ...] seen above).

awaitResolution() (:1772) has the identical shape, calling sendPull(this.importId) without testing this.resolution. It appears unreachable in that state: its only two callers are RpcImportHook.pull(), which returns on entry.resolution one line earlier at :1865, and the sendStream path at :2162, whose entry is constructed with pulling = true so the constructor seeds activePull and the sendPull branch never runs. Guarding it costs one line and cannot be covered by a test for that reason. Worth flagging either way, since the shape remains.

Impact

Six users across macOS and Windows since 2026-07-31, in an app where one session multiplexes every backend service, so a single occurrence costs the user every service at once and forces a reload.

Happy to open a PR if the approach looks right.

主要语言
TypeScript
星标
4k
派生
150
平均合并
3 天 12 小时
30 天内合并 PR
2

环境准备

  • 没有 Dockerfile 或 Docker Compose 文件
  • 没有 Pull Request 模板
  • 阅读贡献指南

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

cloudflare/capnweb 的其他 Issue

查看 cloudflare/capnweb 的全部 Issue

相似的 Issue

更多 TypeScript Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。