cloudflare / cloudflare/capnweb

`ImportTableEntry`'s `remoteRefcount` is only ever 1 or 0

Open
#140 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
4k
Forks
143
Avg merge
4d 6h
Merged PRs (30d)
7

Description

`ImportTableEntry`'s `remoteRefCount` is effectively a boolean.

```console
$ rg remoteRefcount
src/rpc.ts
63: public remoteRefcount: number = 1;
164: if (this.remoteRefcount > 0) {
165: this.session.sendRelease(this.importId, this.remoteRefcount);
166: this.remoteRefcount = 0;
623: // are automatically pulled. Set remoteRefcount to 0 so that resolve() won't send a release
628: entry.remoteRefcount = 0;
633: // (Normally, sendRelease() cleans up the import table, but since remoteRefcount is 0, we
676: sendRelease(id: ImportId, remoteRefcount: number) {
679: this.send(["release", id, remoteRefcount]);
```

The only assignments are the initialization to `1`, and assignments to `0`.

https://github.com/cloudflare/capnweb/blob/c2bb17b940b23eb8ab89be1e85538493cb4552ad/src/rpc.ts#L501-L510
It seems like `importStub` should increment the remoteRefCount when it finds an existing stub, whereas it currently uses the existing `ImportTableEntry`, without incrementing the `remoteRefcount`.

This _would_ lead to a leak, when we eventually send `["release", id, 1]`, we only release one reference, but the peer's export may have `refcount` > 1, however, I believe we're currently never actually using the same export id multiple times. I'll open a separate issue when I finish investigating.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.