CRSessionClient attempts to retain reference to a transferred MessagePort object

[turbocrime]

initial design of @penumbra-zone/transport-chrome and CRSessionClient made the assumption that one chrome.runtime.Port/MessageChannel pair should exist per document lifecycle. this simplified reasoning and implementation, and did not affect actual Transport lifecycle, which is independent of the connection.

this assumption was extended to the initial design of the PenumbraProvider page api of @penumbra-zone/client. But the development of ADR-006 changed this, as api consumers desired multiple lifecycles per page.

in 88b16a7, CRSessionClient was updated minimally to support this, but it seems this minimal change was ultimately incorrect: desiring to re-use the same channel, it now attempts to retain reference to the page's MessagePort which is ultimately transferred out of its isolated content-script execution context and into the mainworld execution context. transferred objects are no longer available in their original context, and any reference will throw on access.

this didn't affect prax's impelementation of ADR-006 directly, but did affect prax's attempt at resolving a backwards compatibility issue prax-wallet/prax#173 (comment)

objectives

don't retain reference to any transferred object

the current implemetation attempts to re-provide the same channel when called again, but this is incorrect and any implementation of that is impossible. consuming content-scripts likewise cannot be responsible for retaining and re-sending the MessagePort when necessary, as this violates the same restriction of transferred objects.

don't support re-providing the same MessagePort

result of the above objective. this will affect implementation of consumer's isolated content scripts, but should be simplifying.

don't require any PenumbraProvider behavioral change

interfaces and behavior defined by ADR-006 should not be affected. unlikely to be a problem.

implementation

there are a few possible implementations. A and B contain potential handler leaks, but solving that as in C requires more extensive lifecycle management which is unclear.

option A: create multiple MessagePorts internally

instead of trying to retain the transferred MessagePort, simply create a new MessageChannel, attach the existing listeners, and transfer a new port.

benefits: remain the same externally.

downsides: intentionally, no routing exists at this level, but now some basic routing must be implemented. potential leak of handlers to unused channels. potential leak of streams buffering to irrelevant channels.

it seems initially simple to just broadcast each response to every channel, as the transport channel clients discard responses labelled with an unrecognized request id. but ReadableStreams are created here, and ultimately transferred out of the execution context - so either the streams must be teed with some kind of cleanup to prevent memory leak, or some basic routing is required.

option B: remove the singleton restriction

instead of trying to retain the transferred MessagePort, only provide it once. require callers to create a new session client to obtain a port again.

beneftis: minimal changes, but not the same externally. instead of conditional init on first request, an isolated content script would init a session client for every connection request.

downsides: it's not currently possible to identify the appropriate end of session client lifetime, so this has the same potential listener leak as A.

option C: option B plus more involved transport-level lifecycle management

this would require changes in @penumbra-zone/transport-dom and the channel-level messaging, so the session client may be aware of the presently connected transports. current expectations of transport use don't provide an obvious way to identify an unused transport.

suggestion

i prefer option B. this reduces new behaviors and code diff, at the cost of a small api diff. with option B, changes are restricted to @penumbra-zone/transport-chrome. its use is purely internal to consumers, so consumers may upgrade the package at their leisure.

its worst case is that a page may make a connection request and a new transport for every method call, and the providing mainworld content-script posts a new connection request to get a new port for each of those.

but users of @penumbra-zone/client are expected to retain a port and not make prolific requests. with the removal of the initial assumptions about channel lifecycle, and the additional state tracking required by ADR-006, providers implementations can correctly understand when to discard/reacquire a port.