8000 Code review cleanup: · coder/coder-desktop-windows@3fc4d6f · GitHub
[go: up one dir, main page]
Skip to content

Commit 3fc4d6f

Browse files
spikecurtisspikecurtis
committed
Code review cleanup:
 * refactor retry logic for StartDaemon * fixup checks for _sessionCount < 0 * fix Vpn targeting windows to avoid complaints about the registry API calxOAOA
1 parent ccfab63 commit 3fc4d6f

File tree

6 files changed

+42
-135
lines changed

6 files changed

+42
-135
lines changed

‎App/Services/MutagenController.cs

Lines changed: 34 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public interface ISyncSessionController
4646
}
4747

4848
// These values are the config option names used in the registry. Any option
49-
// here can be configured with `(Debug)?Mutagen:OptionName` in the registry.
49+
// here can be configured with `(Debug)?MutagenController:OptionName` in the registry.
5050
//
5151
// They should not be changed without backwards compatibility considerations.
5252
// If changed here, they should also be changed in the installer.
@@ -141,7 +141,7 @@ public async Task<List<SyncSession>> ListSyncSessions(CancellationToken ct)
141141
// reads of _sessionCount are atomic, so don't bother locking for this quick check.
142142
switch (_sessionCount)
143143
{
144-
case -1:
144+
case < 0:
145145
throw new InvalidOperationException("Controller must be Initialized first");
146146
case 0:
147147
// If we already know there are no sessions, don't start up the daemon
@@ -162,33 +162,14 @@ public async Task Initialize(CancellationToken ct)
162162
_sessionCount = -2; // in progress
163163
}
164164

165-
const int maxAttempts = 5;
166-
ListResponse? sessions = null;
167-
for (var attempts = 1; attempts <= maxAttempts; attempts++)
165+
var client = await EnsureDaemon(ct);
166+
var sessions = await client.Synchronization.ListAsync(new ListRequest
168167
{
169-
ct.ThrowIfCancellationRequested();
170-
try
168+
Selection = new Selection
171169
{
172-
var client = await EnsureDaemon(ct);
173-
sessions = await client.Synchronization.ListAsync(new ListRequest
174-
{
175-
Selection = new Selection
176-
{
177-
All = true,
178-
},
179-
}, cancellationToken: ct);
180-
}
181-
catch (Exception e) when (e is not OperationCanceledException)
182-
{
183-
if (attempts == maxAttempts)
184-
throw;
185-
// back off a little and try again.
186-
await Task.Delay(100, ct);
187-
continue;
188-
}
189-
190-
break;
191-
}
170+
All = true,
171+
},
172+
}, cancellationToken: ct);
192173

193174
using (_ = await _lock.LockAsync(ct))
194175
{
@@ -211,7 +192,7 @@ public async Task Initialize(CancellationToken ct)
211192

212193
public async Task TerminateSyncSession(SyncSession session, CancellationToken ct)
213194
{
214-
if (_sessionCount == -1) throw new InvalidOperationException("Controller must be Initialized first");
195+
if (_sessionCount < 0) throw new InvalidOperationException("Controller must be Initialized first");
215196
var client = await EnsureDaemon(ct);
216197
// TODO: implement
217198

@@ -272,11 +253,7 @@ private async Task<MutagenClient> EnsureDaemon(CancellationToken ct)
272253
// </summary>
273254
private void RemoveTransition(Task<MutagenClient?> transition)
274255
{
275-
using (_ = _lock.Lock())
276-
{
277-
;
278-
}
279-
256+
using var _ = _lock.Lock();
280257
if (_inProgressTransition == transition) _inProgressTransition = null;
281258
}
282259

@@ -293,9 +270,31 @@ private void RemoveTransition(Task<MutagenClient?> transition)
293270
// Mainline; no daemon running.
294271
}
295272

296-
using (_ = await _lock.LockAsync(ct))
273+
// If we get some failure while creating the log file or starting the process, we'll retry
274+
// it up to 5 times x 100ms. Those issues should resolve themselves quickly if they are
275+
// going to at all.
276+
const int maxAttempts = 5;
277+
ListResponse? sessions = null;
278+
for (var attempts = 1; attempts <= maxAttempts; attempts++)
297279
{
298-
StartDaemonProcessLocked();
280+
ct.ThrowIfCancellationRequested();
281+
try
282+
{
283+
using (_ = await _lock.LockAsync(ct))
284+
{
285+
StartDaemonProcessLocked();
286+
}
287+
}
288+
catch (Exception e) when (e is not OperationCanceledException)
289+
{
290+
if (attempts == maxAttempts)
291+
throw;
292+
// back off a little and try again.
293+
await Task.Delay(100, ct);
294+
continue;
295+
}
296+
297+
break;
299298
}
300299

301300
return await WaitForDaemon(ct);

‎Installer/Program.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -262,9 +262,10 @@ private static int BuildMsiPackage(MsiOptions opts)
262262
@"[INSTALLFOLDER]coder-desktop-service.log"),
263263
new RegValue(RegistryHive, RegistryKey, "Manager:TunnelBinarySignatureSigner", "Coder Technologies Inc."),
264264
new RegValue(RegistryHive, RegistryKey, "Manager:TunnelBinaryAllowVersionMismatch", "false"),
265-
// Add registry values that are consumed by the MutagenController.
266-
new RegValue(RegistryHive, RegistryKey, "MutagenController:MutagenExecutablePath", @"[INSTALLFOLDER]mutagen.exe")
267-
);
265+
// Add registry values that are consumed by the MutagenController. See App/Services/MutagenController.cs
266+
new RegValue(RegistryHive, RegistryKey, "MutagenController:MutagenExecutablePath",
267+
@"[INSTALLFOLDER]mutagen.exe")
268+
);
268269

269270
// Note: most of this control panel info will not be visible as this
270271
// package is usually hidden in favor of the bootstrapper showing

‎Tests.Vpn/packages.lock.json

Lines changed: 1 addition & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -36,38 +36,11 @@
3636
"resolved": "4.6.0",
3737
"contentHash": "R7e1+a4vuV/YS+ItfL7f//rG+JBvVeVLX4mHzFEZo4W1qEKl8Zz27AqvQSAqo+BtIzUCo4aAJMYa56VXS4hudw=="
3838
},
39-
"Google.Protobuf": {
40-
"type": "Transitive",
41-
"resolved": "3.29.3",
42-
"contentHash": "t7nZFFUFwigCwZ+nIXHDLweXvwIpsOXi+P7J7smPT/QjI3EKxnCzTQOhBqyEh6XEzc/pNH+bCFOOSjatrPt6Tw=="
43-
},
4439
"Microsoft.CodeCoverage": {
4540
"type": "Transitive",
4641
"resolved": "17.12.0",
4742
"contentHash": "4svMznBd5JM21JIG2xZKGNanAHNXplxf/kQDFfLHXQ3OnpJkayRK/TjacFjA+EYmoyuNXHo/sOETEfcYtAzIrA=="
4843
},
49-
"Microsoft.Extensions.Configuration": {
50-
"type": "Transitive",
51-
"resolved": "9.0.1",
52-
"contentHash": "VuthqFS+ju6vT8W4wevdhEFiRi1trvQtkzWLonApfF5USVzzDcTBoY3F24WvN/tffLSrycArVfX1bThm/9xY2A==",
53-
"dependencies": {
54-
"Microsoft.Extensions.Configuration.Abstractions": "9.0.1",
55-
"Microsoft.Extensions.Primitives": "9.0.1"
56-
}
57-
},
58-
"Microsoft.Extensions.Configuration.Abstractions": {
59-
"type": "Transitive",
60-
"resolved": "9.0.1",
61-
"contentHash": "+4hfFIY1UjBCXFTTOd+ojlDPq6mep3h5Vq5SYE3Pjucr7dNXmq4S/6P/LoVnZFz2e/5gWp/om4svUFgznfULcA==",
62-
"dependencies": {
63-
"Microsoft.Extensions.Primitives": "9.0.1"
64-
}
65-
},
66-
"Microsoft.Extensions.Primitives": {
67-
"type": "Transitive",
68-
"resolved": "9.0.1",
69-
"contentHash": "bHtTesA4lrSGD1ZUaMIx6frU3wyy0vYtTa/hM6gGQu5QNrydObv8T5COiGUWsisflAfmsaFOe9Xvw5NSO99z0g=="
70-
},
7144
"Microsoft.TestPlatform.ObjectModel": {
7245
"type": "Transitive",
7346
"resolved": "17.12.0",
@@ -90,38 +63,13 @@
9063
"resolved": "13.0.1",
9164
"contentHash": "ppPFpBcvxdsfUonNcvITKqLl3bqxWbDCZIzDWHzjpdAHRFfZe0Dw9HmA0+za13IdyrgJwpkDTDA9fHaxOrt20A=="
9265
},
93-
"Semver": {
94-
"type": "Transitive",
95-
"resolved": "3.0.0",
96-
"contentHash": "9jZCicsVgTebqkAujRWtC9J1A5EQVlu0TVKHcgoCuv345ve5DYf4D1MjhKEnQjdRZo6x/vdv6QQrYFs7ilGzLA==",
97-
"dependencies": {
98-
"Microsoft.Extensions.Primitives": "5.0.1"
99-
}
100-
},
101-
"System.IO.Pipelines": {
102-
"type": "Transitive",
103-
"resolved": "9.0.1",
104-
"contentHash": "uXf5o8eV/gtzDQY4lGROLFMWQvcViKcF8o4Q6KpIOjloAQXrnscQSu6gTxYJMHuNJnh7szIF9AzkaEq+zDLoEg=="
105-
},
10666
"System.Reflection.Metadata": {
10767
"type": "Transitive",
10868
"resolved": "1.6.0",
10969
"contentHash": "COC1aiAJjCoA5GBF+QKL2uLqEBew4JsCkQmoHKbN3TlOZKa2fKLz5CpiRQKDz0RsAOEGsVKqOD5bomsXq/4STQ=="
11070
},
11171
"Coder.Desktop.Vpn": {
112-
"type": "Project",
113-
"dependencies": {
114-
"Coder.Desktop.Vpn.Proto": "[1.0.0, )",
115-
"Microsoft.Extensions.Configuration": "[9.0.1, )",
116-
"Semver": "[3.0.0, )",
117-
"System.IO.Pipelines": "[9.0.1, )"
118-
}
119-
},
120-
"Coder.Desktop.Vpn.Proto": {
121-
"type": "Project",
122-
"dependencies": {
123-
"Google.Protobuf": "[3.29.3, )"
124-
}
72+
"type": "Project"
12573
}
12674
}
12775
}

‎Vpn.DebugClient/packages.lock.json

Lines changed: 1 addition & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -7,49 +7,8 @@
77
"resolved": "3.29.3",
88
"contentHash": "t7nZFFUFwigCwZ+nIXHDLweXvwIpsOXi+P7J7smPT/QjI3EKxnCzTQOhBqyEh6XEzc/pNH+bCFOOSjatrPt6Tw=="
99
},
10-
"Microsoft.Extensions.Configuration": {
11-
"type": "Transitive",
12-
"resolved": "9.0.1",
13-
"contentHash": "VuthqFS+ju6vT8W4wevdhEFiRi1trvQtkzWLonApfF5USVzzDcTBoY3F24WvN/tffLSrycArVfX1bThm/9xY2A==",
14-
"dependencies": {
15-
"Microsoft.Extensions.Configuration.Abstractions": "9.0.1",
16-
"Microsoft.Extensions.Primitives": "9.0.1"
17-
}
18-
},
19-
"Microsoft.Extensions.Configuration.Abstractions": {
20-
"type": "Transitive",
21-
"resolved": "9.0.1",
22-
"contentHash": "+4hfFIY1UjBCXFTTOd+ojlDPq6mep3h5Vq5SYE3Pjucr7dNXmq4S/6P/LoVnZFz2e/5gWp/om4svUFgznfULcA==",
23-
"dependencies": {
24-
"Microsoft.Extensions.Primitives": "9.0.1"
25-
}
26-
},
27-
"Microsoft.Extensions.Primitives": {
28-
"type": "Transitive",
29-
"resolved": "9.0.1",
30-
"contentHash": "bHtTesA4lrSGD1ZUaMIx6frU3wyy0vYtTa/hM6gGQu5QNrydObv8T5COiGUWsisflAfmsaFOe9Xvw5NSO99z0g=="
31-
},
32-
"Semver": {
33-
"type": "Transitive",
34-
"resolved": "3.0.0",
35-
"contentHash": "9jZCicsVgTebqkAujRWtC9J1A5EQVlu0TVKHcgoCuv345ve5DYf4D1MjhKEnQjdRZo6x/vdv6QQrYFs7ilGzLA==",
36-
"dependencies": {
37-
"Microsoft.Extensions.Primitives": "5.0.1"
38-
}
39-
},
40-
"System.IO.Pipelines": {
41-
"type": "Transitive",
42-
"resolved": "9.0.1",
43-
"contentHash": "uXf5o8eV/gtzDQY4lGROLFMWQvcViKcF8o4Q6KpIOjloAQXrnscQSu6gTxYJMHuNJnh7szIF9AzkaEq+zDLoEg=="
44-
},
4510
"Coder.Desktop.Vpn": {
46-
"type": "Project",
47-
"dependencies": {
48-
"Coder.Desktop.Vpn.Proto": "[1.0.0, )",
49-
"Microsoft.Extensions.Configuration": "[9.0.1, )",
50-
"Semver": "[3.0.0, )",
51-
"System.IO.Pipelines": "[9.0.1, )"
52-
}
11+
"type": "Project"
5312
},
5413
"Coder.Desktop.Vpn.Proto": {
5514
"type": "Project",

‎Vpn/Vpn.csproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
<PropertyGroup>
44
<AssemblyName>Coder.Desktop.Vpn</AssemblyName>
55
<RootNamespace>Coder.Desktop.Vpn</RootNamespace>
6-
<TargetFramework>net8.0</TargetFramework>
6+
<TargetFramework>net8.0-windows</TargetFramework>
77
<ImplicitUsings>enable</ImplicitUsings>
88
<Nullable>enable</Nullable>
99
<RestorePackagesWithLockFile>true</RestorePackagesWithLockFile>

‎Vpn/packages.lock.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"version": 1,
33
"dependencies": {
4-
"net8.0": {
4+
"net8.0-windows7.0": {
55
"Microsoft.Extensions.Configuration": {
66
"type": "Direct",
77
"requested": "[9.0.1, )",

0 commit comments

Comments
 (0)
0