- Notifications
You must be signed in to change notification settings - Fork3
fix: various fixes for tunnel startup to work#14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Fixes various problems that prevented tunnel startup from succeeding andprevented clients from connecting to the RPC server.Added a new temporary package Vpn.DebugClient to help control the systemservice component until the UI is ready.Adds utility PS1 scripts for adding the debug binary to a new systemservice and managing/hotswapping it.
Uh oh!
There was an error while loading.Please reload this page.
Vpn.Service/Manager.cs Outdated
await DownloadTunnelBinaryAsync(message.Message.Start.CoderUrl, serverVersion, ct); | ||
await _tunnelSupervisor.StartAsync(_config.TunnelBinaryPath, HandleTunnelRpcMessage, | ||
HandleTunnelRpcError, | ||
await _tunnelSupervisor.StartAsync(_config.TunnelBinaryPath, HandleTunnelRpcMessage, HandleTunnelRpcError, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
same with this section: we need to ensure that stops and starts are serialized so that you don't getstop, stop, start, start
if two clients connect around the same time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Added a semaphore that fixes both of these issues
public async Task BroadcastAsync(ServiceMessage message, CancellationToken ct) | ||
{ | ||
foreach (var (clientId, client) in _activeClients) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Is there anything that prevents_activeClients
from being modified while you are looping over it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Since it's a concurrent dictionary it's exception-safe. As far as I understand, any added or removed items from the dictionary while we're looping may or may not get called, but I think that's fine.
4c6d2bd
intomainUh oh!
There was an error while loading.Please reload this page.
Fixes various problems that prevented tunnel startup from succeeding and prevented clients from connecting to the RPC server.
Added a new temporary package Vpn.DebugClient to help control the system service component until the UI is ready.
Adds utility PS1 scripts for adding the debug binary to a new system service and managing/hotswapping it.
Start and stop operations from a client now reliably work, although there are various TODOs around the service codebase still.
Closes#2