Skip to content

ImportTableEntry's remoteRefcount is only ever 1 or 0 #140

@Dr-Emann

Description

@Dr-Emann

ImportTableEntry's remoteRefCount is effectively a boolean.

$ 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.

capnweb/src/rpc.ts

Lines 501 to 510 in c2bb17b

importStub(idx: ImportId): RpcImportHook {
if (this.abortReason) throw this.abortReason;
let entry = this.imports[idx];
if (!entry) {
entry = new ImportTableEntry(this, idx, false);
this.imports[idx] = entry;
}
return new RpcImportHook(/*isPromise=*/false, entry);
}

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions