Simplify code around latency detection for desktop

[probakowski]

This change reduces number of channels and goroutines we use to move messages around in proxy and makes tdp.Conn and tdp.ConnProxy more thread-safe by guarding writes with mutex

[gzdunek]

Aren’t we missing context cancellation handling in the goroutines? Something like

Suggested change

	for {
	msg, err := c.client.ReadMessage()
	if err := c.handleError(err); err != nil {
	return err
	}
	if err := c.SendToServer(msg); err != nil {
	return trace.Wrap(err)
	}
	}
	for {
	select {
	case <-ctx.Done():
	return ctx.Err()
	default:
	msg, err := c.client.ReadMessage()
	if err := c.handleError(err); err != nil {
	return err
	}
	if err := c.SendToServer(msg); err != nil {
	return trace.Wrap(err)
	}
	}
	}

[probakowski]

Added

[espadolini]

Don't mix context cancellation with IO like that, it doesn't do anything and just adds to the confusion. If the loop should end when the context is cancelled then c.client.ReadMessage() and c.SendToServer() should unblock and return an error when the context is cancelled, and if they don't then this is just occasionally succeeding in exiting the loop when the context is done if we get lucky with the timing, and a possible deadlock otherwise.

[espadolini]

To clarify, if cancelling the context closes the connections then there's nothing to be added by checking for the context in this loop, and if cancelling the context does not close the connections then we are going to block on read or write and we will not be respecting context cancellation.

[probakowski]

So ctx here is connected with client side of the connection so its cancellation should mean that client will close as well. That means its goroutine will exit and in turn close server side (with closeAll), that will end the other goroutine as well. I removed the ctx.Err check.