Support multiplexing of spawned debug sessions

[connor4312]

From #79, we now have a way for DA's to start client sessions, which spawns new DA's.

However, for many cases, multiplexing the new sessions over the same DA connection is preferable. As a starting point, I suggest the following.

First, to modify StartDebuggingRequestArguments to accept a connectionId?: number.

interface StartDebuggingRequestArguments {
  /**
   * Requests that the new connection be multiplexed over the same transport
   * as the parent session. If present, ProtocolMessages for the new session 
   * will have a corresponding `connectionId` property. It is not valid to use
   * an ID already in use by an existing session.
   */
  connectionId?: number;

Then to add a corresponding optional property on the protocol message.

interface ProtocolMessage {
  /**
   * If the current session was a child started via a `startDebugging` request, 
   * then this is set to the `connectionId` that was present in the `StartDebuggingRequestArguments`.
   */
  connectionId?: number;

And of course add a capability in the InitializeRequestArguments so DA's know whether this is supported

interface InitializeRequestArguments {
  /**
   * Whether the client supports `connectionId` in `StartDebuggingRequestArguments` 
   */
  supportsConnectionMultiplexing?: boolean;

[roblourens]

Guessing you meant the capability to be under interface InitializeRequestArguments {

[connor4312]

Yep, fixed

[gfszr]

There was a lot of discussion in #79 about specifying how the client should now how to connect to the new debugging session, so this solution actually very much eases how to do so and should be the standard way to actually implement startDebugging requests.

I do think however that session multiplexing has other benefits which might be useful in the future, and binding the ability to multiplex connections to startDebuggingRequest by name will not be future compatible to such possible future benefits.

@connor4312 How about renaming supportsConnectionIdInStartDebuggingRequest to something more generic, such as supportsConnectionMultiplexing, by which connectionId is the sole ID used for connection multiplexing (Maybe it can be renamed too to something more specific, like connectionMultiplexId?)

[gregg-miskelly]

My two cents worth: I agree that ProtocolMessage.connectionId is potentially useful in other scenarios where maybe one wants a single logical pipe to handle multiple sessions. So, I wonder if the description for connectionId there should be more generic. But I don't see how the client capability could be used outside of startDebuggingRequest, so that one seems fine to me.

[weinand]

I wonder how the multiplexing feature would be supported in VS Code. Today VS Code delegates debug session creation to the adapter factories inside the extension host. Only that factory would know how to support multiplexing. That means VS Code does not know anything about the multiplexing, and as a consequence it could only provide the supportsConnectionIdInStartDebuggingRequest capability after talking to the extension through some yet to be defined debug extension API, right?

BTW, just some additional architectural data point:
When using the VS Code's "Remote Development" extensions (SSH, Container, WSL) VS Code's DAP streams are already multiplexed over the communication channel that exists between VS Code and the remote extension host. Here the multiplexing happens on a different layer ("extension host RPC") and the individual DAP messages are just wrapped and not modified by an embedded connectionId.

[connor4312]

In VS Code, for a multiplexed session I would think that we don't need to touch the DebugAdapterDescriptorFactory -- we would already have a descriptor from the first session, and send data over whatever connection or set of methods we were already using to communicate.

How about renaming supportsConnectionIdInStartDebuggingRequest to something more generic, such as supportsConnectionMultiplexing, by which connectionId is the sole ID used for connection multiplexing (Maybe it can be renamed too to something more specific, like connectionMultiplexId?)

👍 for the capability name. I would prefer to keep the connectionId property shorter, though, since that is sent as overhead in every DAP message

[gregg-miskelly]

I wonder how the multiplexing feature would be supported in VS Code. Today VS Code delegates debug session creation to the adapter factories inside the extension host. Only that factory would know how to support multiplexing.

My suggestions:

• For the startDebuggingRequest case, if StartDebuggingRequestArguments.connectionId is specified, then I would agree with Connor's suggestion of using the existing descriptor.
• For attach/launches initiated without a startDebuggingRequest -- potentially DebugAdapterExecutableFactory could have a new property to indicate that the debug adapter should be a singleton, so if another debug session was started while the debug adapter is still running, reuse it rather than spawning another process.

[weinand]

• if StartDebuggingRequestArguments.connectionId is specified, then the DebugAdapterDescriptorFactory can be completely ignored because it only has a factory method that is of no use for multiplexing. Instead the communication channel established by the initial factory must be reused. This will require that the life cycle of the debug sessions must be cleanly nested: no parent is allowed to terminate before its child sessions because otherwise the shared communication channel would go away too early.
• For attach/launches initiated without a startDebuggingRequest I do not see where the "singleton" decision is being made. A client cannot really decide whether unrelated debug sessions can be multiplexed through the "first" session.

[gregg-miskelly]

This will require that the life cycle of the debug sessions must be cleanly nested: no parent is allowed to terminate before its child sessions because otherwise the shared communication channel would go away too early.

To be clear: this isn't a strict requirement -- the client could keep a counter of the number of active debug sessions be shared by a connection and only break it when the counter reached zero.

• For attach/launches initiated without a startDebuggingRequest I do not see where the "singleton" decision is being made. A client cannot really decide whether unrelated debug sessions can be multiplexed through the "first" session.

This part is client-specific. As I said above, if VS Code wanted to support singleton debug adapters, I think the logical way would be to extend DebugAdapterExecutableFactory with a property indicating that the debug adapter would like to be treated as singleton. In case you are wondering why anyone would want this: it could potentially be very useful for performance if the debug adapter was implemented in a language that uses JIT compilation since now the JIT compilation is only happening in the initial debug session rather than every debug session.