`exportStub()` always allocates a new export ID, even for duplicate capabilities

[Dr-Emann]

exportStub() deduplication by hook identity never fires.

capnweb/src/rpc.ts

Lines 369 to 383 in c2bb17b

	exportStub(hook: StubHook): ExportId {
	if (this.abortReason) throw this.abortReason;
	let existingExportId = this.reverseExports.get(hook);
	if (existingExportId !== undefined) {
	++this.exports[existingExportId].refcount;
	return existingExportId;
	} else {
	let exportId = this.nextExportId--;
	this.exports[exportId] = { hook, refcount: 1 };
	this.reverseExports.set(hook, exportId);
	// TODO: Use onBroken().
	return exportId;
	}
	}

reverseExports is a Map<StubHook, ExportId>, so dedup is by object identity.

capnweb/src/serialize.ts

Lines 275 to 281 in c2bb17b

	private devaluateHook(type: "export" | "promise" | "writable", hook: StubHook): unknown {
	if (!this.exports) this.exports = [];
	let exportId = type === "promise" ? this.exporter.exportPromise(hook)
	: this.exporter.exportStub(hook);
	this.exports.push(exportId);
	return [type, exportId];
	}

devaluateHook routes to either exportStub or exportPromise depending on whether the type is "export"/"writable" or "promise". The promise path is intentionally never deduplicated (exportPromise comment: "Promises always use a new ID because otherwise the recipient could miss the resolution."), so the issue only concerns the paths that reach exportStub:

"stub"/"rpc-promise"

capnweb/src/serialize.ts

Lines 218 to 224 in c2bb17b

	if (pathIfPromise) {
	hook = hook.get(pathIfPromise);
	} else {
	hook = hook.dup();
	}
	return this.devaluateHook(pathIfPromise ? "promise" : "export", hook);

hook is set to the result of hook.dup() if pathIfPromise is falsy (the only time exportStub will be called).

"function"/"rpc-target"

capnweb/src/serialize.ts

Lines 233 to 234 in c2bb17b

	let hook = this.source.getHookForRpcTarget(<RpcTarget|Function>value, parent);
	return this.devaluateHook("export", hook);

getHookForRpcTarget() returns hook.dup() for "return" payloads (after correctly deduplicating in rpcTargets by target object identity), or a fresh TargetStubHook.create() each time for "params" payloads. Either way, a fresh object reaches exportStub.

"writable"

capnweb/src/serialize.ts

Lines 251 to 252 in c2bb17b

	let hook = this.source.getHookForWritableStream(<WritableStream>value, parent);
	return this.devaluateHook("writable", hook);

getHookForWritableStream() uses identical logic to getHookForRpcTarget() for our purposes, it deduplicates in rpcTargets for "return" payloads but returns hook.dup() each time, fresh WritableStreamStubHook each time for "params".

In all three cases, dup() always returns a new object for every hook type that can be exported (RpcImportHook, TargetStubHook, WritableStreamStubHook, ReadableStreamStubHook, PayloadStubHook). The two types where dup() returns this (ErrorStubHook and MapVariableHook) are never exported.

Because the stub fed to exportStub is always a newly created object, it will never be found in reverseExports, so we will always create a new export.

This means the same capability appearing twice always gets separate export IDs. This is what prevents the remoteRefcount bug (#140) from being triggered currently