Skip to content

Commit ee41d4a

Browse files
danmoseleydanmoseley
authored
Fix TestOnStartWithArgsThenStop and make service tests more reliable (#39153)
* Comment dispose * Fix service tests * typo
1 parent 7f7e795 commit ee41d4a

File tree

6 files changed

+74
-23
lines changed

6 files changed

+74
-23
lines changed

‎src/libraries/System.ServiceProcess.ServiceController/src/System/ServiceProcess/ServiceBase.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,8 @@ internal static bool ValidServiceName(string serviceName)
305305
/// <devdoc>
306306
/// <para>Disposes of the resources (other than memory ) used by
307307
/// the <see cref='System.ServiceProcess.ServiceBase'/>.</para>
308+
/// This is called from <see cref="Run(ServiceBase[])"/> when all
309+
/// services in the process have entered the SERVICE_STOPPED state.
308310
/// </devdoc>
309311
protected override void Dispose(bool disposing)
310312
{

‎src/libraries/System.ServiceProcess.ServiceController/tests/ServiceBaseTests.cs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,17 +84,21 @@ public void TestOnStartThenStop()
8484
public void TestOnStartWithArgsThenStop()
8585
{
8686
ServiceController controller = ConnectToServer();
87-
8887
controller.Stop();
8988
Assert.Equal((int)PipeMessageByteCode.Stop, _testService.GetByte());
9089
controller.WaitForStatus(ServiceControllerStatus.Stopped);
9190

9291
controller.Start(new string[] { "StartWithArguments", "a", "b", "c" });
92+
93+
// Start created a new TestService; dispose of our client stream and reconnect to it
9394
_testService.Client = null;
9495
_testService.Client.Connect();
95-
Assert.Equal((int)(PipeMessageByteCode.Connected), _testService.GetByte());
9696

97-
Assert.Equal((int)(PipeMessageByteCode.Start), _testService.GetByte());
97+
// Test service does not mutually synchronize Connected and Start messages
98+
var bytes = new byte[] { _testService.GetByte(), _testService.GetByte() };
99+
Assert.Contains((byte)PipeMessageByteCode.Connected, bytes);
100+
Assert.Contains((byte)PipeMessageByteCode.Start, bytes);
101+
98102
controller.WaitForStatus(ServiceControllerStatus.Running);
99103

100104
controller.Stop();
@@ -202,8 +206,10 @@ public void PropagateExceptionFromOnStart()
202206

203207
private ServiceController ConnectToServer()
204208
{
209+
TestServiceProvider.DebugTrace("ServiceBaseTests.ConnectToServer: connecting");
205210
_testService.Client.Connect(connectionTimeout);
206211
Assert.Equal((int)PipeMessageByteCode.Connected, _testService.GetByte());
212+
TestServiceProvider.DebugTrace("ServiceBaseTests.ConnectToServer: received connect byte");
207213

208214
ServiceController controller = new ServiceController(_testService.TestServiceName);
209215
AssertExpectedProperties(controller);

‎src/libraries/System.ServiceProcess.ServiceController/tests/System.ServiceProcess.ServiceController.TestService/Program.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Threading;
5+
46
namespace System.ServiceProcess.Tests
57
{
68
public class Program
79
{
810
static int Main(string[] args)
911
{
12+
Thread.CurrentThread.Name = "Test Service";
1013
if (args.Length == 1 || args.Length == 2)
1114
{
1215
TestService testService;

‎src/libraries/System.ServiceProcess.ServiceController/tests/System.ServiceProcess.ServiceController.TestService/TestService.cs

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,19 @@ namespace System.ServiceProcess.Tests
99
{
1010
public class TestService : ServiceBase
1111
{
12+
// To view tracing, use DbgView from sysinternals.com;
13+
// run it elevated, check "Capture>Global Win32" and "Capture>Win32",
14+
// and filter to just messages beginning with "##"
15+
internal const bool DebugTracing = false;
16+
1217
private bool _disposed;
1318
private Task _waitClientConnect;
1419
private NamedPipeServerStream _serverStream;
1520
private readonly Exception _exception;
1621

1722
public TestService(string serviceName, Exception throwException = null)
1823
{
24+
DebugTrace("TestService " + ServiceName + ": Ctor");
1925
this.ServiceName = serviceName;
2026

2127
// Enable all the events
@@ -27,10 +33,11 @@ public TestService(string serviceName, Exception throwException = null)
2733
this.CanHandleSessionChangeEvent = false;
2834
this.CanHandlePowerEvent = false;
2935
this._exception = throwException;
30-
3136
this._serverStream = new NamedPipeServerStream(serviceName);
3237
_waitClientConnect = this._serverStream.WaitForConnectionAsync();
33-
_waitClientConnect.ContinueWith((t) => WriteStreamAsync(PipeMessageByteCode.Connected));
38+
_waitClientConnect = _waitClientConnect.ContinueWith(_ => DebugTrace("TestService " + ServiceName + ": Connected"));
39+
_waitClientConnect.ContinueWith(t => WriteStreamAsync(PipeMessageByteCode.Connected));
40+
DebugTrace("TestService " + ServiceName + ": Ctor completed");
3441
}
3542

3643
protected override void OnContinue()
@@ -72,6 +79,7 @@ protected override void OnShutdown()
7279

7380
protected override void OnStart(string[] args)
7481
{
82+
DebugTrace("TestService " + ServiceName + ": OnStart");
7583
base.OnStart(args);
7684
if (_exception != null)
7785
{
@@ -89,41 +97,52 @@ protected override void OnStart(string[] args)
8997

9098
protected override void OnStop()
9199
{
100+
DebugTrace("TestService " + ServiceName + ": OnStop");
92101
base.OnStop();
93102
WriteStreamAsync(PipeMessageByteCode.Stop).Wait();
94103
}
95104

96105
public async Task WriteStreamAsync(PipeMessageByteCode code, int command = 0)
97106
{
98-
if (_waitClientConnect.IsCompleted)
99-
{
100-
Task writeCompleted;
101-
const int WriteTimeout = 60000;
102-
if (code == PipeMessageByteCode.OnCustomCommand)
103-
{
104-
writeCompleted = _serverStream.WriteAsync(new byte[] { (byte)command }, 0, 1);
105-
}
106-
else
107-
{
108-
writeCompleted = _serverStream.WriteAsync(new byte[] { (byte)code }, 0, 1);
109-
}
110-
await writeCompleted.TimeoutAfter(WriteTimeout).ConfigureAwait(false);
111-
}
112-
else
107+
DebugTrace("TestService " + ServiceName + ": WriteStreamAsync writing " + code.ToString());
108+
109+
const int WriteTimeout = 60000;
110+
var toWrite = (code == PipeMessageByteCode.OnCustomCommand) ? new byte[] { (byte)command } : new byte[] { (byte)code };
111+
112+
// Wait for the client connection before writing to the pipe.
113+
// Exception: if it's a Stop message, it may be because the test has completed and we're cleaning up the test service so there's no client at all.
114+
// Tests that verify "Stop" itself should ensure the client connection has completed before calling stop, by waiting on some other message from the pipe first.
115+
if (code != PipeMessageByteCode.Stop)
113116
{
114-
// We get here if the service is getting torn down before a client ever connected.
115-
// some tests do this.
117+
await _waitClientConnect;
116118
}
119+
await _serverStream.WriteAsync(toWrite, 0, 1).TimeoutAfter(WriteTimeout).ConfigureAwait(false);
120+
DebugTrace("TestService " + ServiceName + ": WriteStreamAsync completed");
117121
}
118122

123+
/// <summary>
124+
/// This is called from <see cref="ServiceBase.Run(ServiceBase[])"/> when all services in the process
125+
/// have entered the SERVICE_STOPPED state. It disposes the named pipe stream.
126+
/// </summary>
119127
protected override void Dispose(bool disposing)
120128
{
121129
if (!_disposed)
122130
{
131+
DebugTrace("TestService " + ServiceName + ": Disposing");
123132
_serverStream.Dispose();
124133
_disposed = true;
125134
base.Dispose();
126135
}
127136
}
137+
138+
internal static void DebugTrace(string message)
139+
{
140+
if (DebugTracing)
141+
{
142+
#pragma warning disable CS0162 // unreachable code
143+
Debug.WriteLine("## " + message);
144+
#pragma warning restore
145+
}
146+
}
128147
}
129148
}

‎src/libraries/System.ServiceProcess.ServiceController/tests/System.ServiceProcess.ServiceController.TestService/TestServiceInstaller.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ public unsafe void Install()
102102
{
103103
if (svc.Status != ServiceControllerStatus.Running)
104104
{
105+
TestService.DebugTrace("TestServiceInstaller: instructing ServiceController to Start service " + ServiceName);
105106
svc.Start();
106107
if (!ServiceName.StartsWith("PropagateExceptionFromOnStart"))
107108
svc.WaitForStatus(ServiceControllerStatus.Running, TimeSpan.FromSeconds(120));
@@ -124,7 +125,6 @@ public void RemoveService()
124125
// Meantime we still want this service to get deleted, so we'll go ahead and call
125126
// DeleteService, which will schedule it to get deleted on reboot.
126127
// We won't catch the exception: we do want the test to fail.
127-
128128
DeleteService();
129129

130130
ServiceName = null;
@@ -141,6 +141,7 @@ private void StopService()
141141
{
142142
try
143143
{
144+
TestService.DebugTrace("TestServiceInstaller: instructing ServiceController to Stop service " + ServiceName);
144145
svc.Stop();
145146
}
146147
catch (InvalidOperationException)
@@ -172,6 +173,7 @@ private void DeleteService()
172173
if (serviceHandle.IsInvalid)
173174
throw new Win32Exception($"Could not find service '{ServiceName}'");
174175

176+
TestService.DebugTrace("TestServiceInstaller: instructing ServiceController to Delete service " + ServiceName);
175177
if (!Interop.Advapi32.DeleteService(serviceHandle))
176178
{
177179
throw new Win32Exception($"Could not delete service '{ServiceName}'");

‎src/libraries/System.ServiceProcess.ServiceController/tests/TestServiceProvider.cs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@ namespace System.ServiceProcess.Tests
1010
{
1111
internal sealed class TestServiceProvider
1212
{
13+
// To view tracing, use DbgView from sysinternals.com;
14+
// run it elevated, check "Capture>Global Win32" and "Capture>Win32",
15+
// and filter to just messages beginning with "##"
16+
internal const bool DebugTracing = false;
17+
1318
private const int readTimeout = 60000;
1419

1520
private static readonly Lazy<bool> s_runningWithElevatedPrivileges = new Lazy<bool>(
@@ -28,6 +33,7 @@ public NamedPipeClientStream Client
2833
{
2934
if (_client == null)
3035
{
36+
DebugTrace("TestServiceProvider: Creating client stream");
3137
_client = new NamedPipeClientStream(".", TestServiceName, PipeDirection.In);
3238
}
3339
return _client;
@@ -36,10 +42,12 @@ public NamedPipeClientStream Client
3642
{
3743
if (value == null)
3844
{
45+
DebugTrace("TestServiceProvider: Disposing client stream");
3946
_client.Dispose();
4047
_client = null;
4148
}
4249
}
50+
4351
}
4452

4553
public readonly string TestServiceAssembly = typeof(TestService).Assembly.Location;
@@ -125,6 +133,7 @@ public void DeleteTestServices()
125133
{
126134
if (_client != null)
127135
{
136+
DebugTrace("TestServiceProvider: Disposing client stream in Dispose()");
128137
_client.Dispose();
129138
_client = null;
130139
}
@@ -143,5 +152,15 @@ public void DeleteTestServices()
143152
}
144153
}
145154
}
155+
156+
internal static void DebugTrace(string message)
157+
{
158+
if (DebugTracing)
159+
{
160+
#pragma warning disable CS0162 // unreachable code
161+
Debug.WriteLine("## " + message);
162+
#pragma warning restore
163+
}
164+
}
146165
}
147166
}

0 commit comments

Comments
 (0)