Skip to content

Commit 0a99919

Browse files
kouvelstephentoub
kouvel
authored and
stephentoub
committed
Fix some monitor tests, add one for lock wait paths (dotnet#28442)
- The monitor tests were not running because it was not a public class, fixed - Fixed some tests, for instance some argument exceptions could be improved to include parameter name (this issue exists elsewhere too), for now I have just changed the tests to reflect current coreclr/desktop behavior - Added a test to hit spinning/waiting paths for thin lock and aware lock
1 parent 51f05b6 commit 0a99919

File tree

1 file changed

+210
-27
lines changed

1 file changed

+210
-27
lines changed

‎src/System.Threading/tests/MonitorTests.cs

+210-27
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,20 @@
33
// See the LICENSE file in the project root for more information.
44

55
using System;
6+
using System.Collections.Generic;
67
using System.Threading;
78
using System.Threading.Tasks;
89
using Xunit;
910

1011
namespace System.Threading.Tests
1112
{
12-
static partial class MonitorTests
13+
public static class MonitorTests
1314
{
1415
private const int FailTimeoutMilliseconds = 30000;
1516

1617
// Attempts a single recursive acquisition/release cycle of a newly-created lock.
1718
[Fact]
18-
public static void BasicRecursion(ref string message)
19+
public static void BasicRecursion()
1920
{
2021
var obj = new object();
2122
Assert.True(Monitor.TryEnter(obj));
@@ -32,7 +33,7 @@ public static void BasicRecursion(ref string message)
3233

3334
// Attempts to overflow the recursion count of a newly-created lock.
3435
[Fact]
35-
public static void DeepRecursion(ref string message)
36+
public static void DeepRecursion()
3637
{
3738
var obj = new object();
3839
var hc = obj.GetHashCode();
@@ -51,7 +52,7 @@ public static void DeepRecursion(ref string message)
5152
}
5253

5354
Monitor.Exit(obj);
54-
Assert.True(Monitor.IsEntered(obj));
55+
Assert.False(Monitor.IsEntered(obj));
5556
}
5657

5758
[Fact]
@@ -98,7 +99,7 @@ public static void Enter_SetsLockTaken()
9899
Monitor.Enter(obj, ref lockTaken);
99100
Assert.True(lockTaken);
100101
Monitor.Exit(obj);
101-
Assert.False(lockTaken);
102+
Assert.True(lockTaken);
102103
}
103104

104105
[Fact]
@@ -107,21 +108,21 @@ public static void Enter_Invalid()
107108
bool lockTaken = false;
108109
var obj = new object();
109110

110-
AssertExtensions.Throws<ArgumentNullException>("obj", () => Monitor.Enter(null));
111-
AssertExtensions.Throws<ArgumentNullException>("obj", () => Monitor.Enter(null, ref lockTaken));
111+
Assert.Throws<ArgumentNullException>(() => Monitor.Enter(null));
112+
Assert.Throws<ArgumentNullException>(() => Monitor.Enter(null, ref lockTaken));
112113
Assert.False(lockTaken);
113114

114115
lockTaken = true;
115116
AssertExtensions.Throws<ArgumentException>("lockTaken", () => Monitor.Enter(obj, ref lockTaken));
116-
Assert.False(lockTaken);
117+
Assert.True(lockTaken);
117118
}
118119

119120
[Fact]
120121
public static void Exit_Invalid()
121122
{
122123
var obj = new object();
123124
int valueType = 1;
124-
AssertExtensions.Throws<ArgumentNullException>("obj", () => Monitor.Exit(null));
125+
Assert.Throws<ArgumentNullException>(() => Monitor.Exit(null));
125126

126127
Assert.Throws<SynchronizationLockException>(() => Monitor.Exit(obj));
127128
Assert.Throws<SynchronizationLockException>(() => Monitor.Exit(new object()));
@@ -180,7 +181,7 @@ public static void TryEnter_SetsLockTaken()
180181
Monitor.TryEnter(obj, ref lockTaken);
181182
Assert.True(lockTaken);
182183
Monitor.Exit(obj);
183-
Assert.False(lockTaken);
184+
Assert.True(lockTaken);
184185
}
185186

186187
[Fact]
@@ -189,35 +190,217 @@ public static void TryEnter_Invalid()
189190
bool lockTaken = false;
190191
var obj = new object();
191192

192-
AssertExtensions.Throws<ArgumentNullException>("obj", () => Monitor.TryEnter(null));
193-
AssertExtensions.Throws<ArgumentNullException>("obj", () => Monitor.TryEnter(null, ref lockTaken));
194-
AssertExtensions.Throws<ArgumentNullException>("obj", () => Monitor.TryEnter(null, 1));
195-
AssertExtensions.Throws<ArgumentNullException>("obj", () => Monitor.TryEnter(null, 1, ref lockTaken));
196-
AssertExtensions.Throws<ArgumentNullException>("obj", () => Monitor.TryEnter(null, TimeSpan.Zero));
197-
AssertExtensions.Throws<ArgumentNullException>("obj", () => Monitor.TryEnter(null, TimeSpan.Zero, ref lockTaken));
193+
Assert.Throws<ArgumentNullException>(() => Monitor.TryEnter(null));
194+
Assert.Throws<ArgumentNullException>(() => Monitor.TryEnter(null, ref lockTaken));
195+
Assert.Throws<ArgumentNullException>(() => Monitor.TryEnter(null, 1));
196+
Assert.Throws<ArgumentNullException>(() => Monitor.TryEnter(null, 1, ref lockTaken));
197+
Assert.Throws<ArgumentNullException>(() => Monitor.TryEnter(null, TimeSpan.Zero));
198+
Assert.Throws<ArgumentNullException>(() => Monitor.TryEnter(null, TimeSpan.Zero, ref lockTaken));
198199

199-
AssertExtensions.Throws<ArgumentOutOfRangeException>("millisecondsTimeout", () => Monitor.TryEnter(null, -1));
200-
AssertExtensions.Throws<ArgumentOutOfRangeException>("millisecondsTimeout", () => Monitor.TryEnter(null, -1, ref lockTaken));
201-
AssertExtensions.Throws<ArgumentOutOfRangeException>("timeout", () => Monitor.TryEnter(null, TimeSpan.FromMilliseconds(-1)));
202-
AssertExtensions.Throws<ArgumentOutOfRangeException>("timeout", () => Monitor.TryEnter(null, TimeSpan.FromMilliseconds(-1), ref lockTaken));
200+
Assert.Throws<ArgumentOutOfRangeException>(() => Monitor.TryEnter(obj, -2));
201+
Assert.Throws<ArgumentOutOfRangeException>(() => Monitor.TryEnter(obj, -2, ref lockTaken));
202+
AssertExtensions.Throws<ArgumentOutOfRangeException>("timeout", () => Monitor.TryEnter(obj, TimeSpan.FromMilliseconds(-2)));
203+
AssertExtensions.Throws<ArgumentOutOfRangeException>("timeout", () => Monitor.TryEnter(obj, TimeSpan.FromMilliseconds(-2), ref lockTaken));
203204

204205
lockTaken = true;
205206
AssertExtensions.Throws<ArgumentException>("lockTaken", () => Monitor.TryEnter(obj, ref lockTaken));
206-
lockTaken = true;
207+
Assert.True(lockTaken);
207208
AssertExtensions.Throws<ArgumentException>("lockTaken", () => Monitor.TryEnter(obj, 0, ref lockTaken));
208-
lockTaken = true;
209+
Assert.True(lockTaken);
209210
AssertExtensions.Throws<ArgumentException>("lockTaken", () => Monitor.TryEnter(obj, TimeSpan.Zero, ref lockTaken));
210211
}
211212

213+
[Fact]
214+
public static void Enter_HasToWait()
215+
{
216+
var thinLock = new object();
217+
var awareLock = new object();
218+
219+
// Actually transition the aware lock to an aware lock by having a background thread wait for a lock
220+
{
221+
Action waitForThread;
222+
Thread t =
223+
ThreadTestHelpers.CreateGuardedThread(out waitForThread, () =>
224+
Assert.False(Monitor.TryEnter(awareLock, ThreadTestHelpers.ExpectedTimeoutMilliseconds)));
225+
t.IsBackground = true;
226+
lock (awareLock)
227+
{
228+
t.Start();
229+
waitForThread();
230+
}
231+
}
232+
233+
// When the current thread has the lock, have background threads wait for the lock in various ways. After a short
234+
// duration, release the lock and allow the background threads to acquire the lock.
235+
{
236+
var backgroundTestDelegates = new List<Action<object>>();
237+
Barrier readyBarrier = null;
238+
239+
backgroundTestDelegates.Add(lockObj =>
240+
{
241+
readyBarrier.SignalAndWait();
242+
Monitor.Enter(lockObj);
243+
Monitor.Exit(lockObj);
244+
});
245+
246+
backgroundTestDelegates.Add(lockObj =>
247+
{
248+
readyBarrier.SignalAndWait();
249+
bool lockTaken = false;
250+
Monitor.Enter(lockObj, ref lockTaken);
251+
Assert.True(lockTaken);
252+
Monitor.Exit(lockObj);
253+
});
254+
255+
backgroundTestDelegates.Add(lockObj =>
256+
{
257+
readyBarrier.SignalAndWait();
258+
lock (lockObj)
259+
{
260+
}
261+
});
262+
263+
backgroundTestDelegates.Add(lockObj =>
264+
{
265+
readyBarrier.SignalAndWait();
266+
Assert.True(Monitor.TryEnter(lockObj, ThreadTestHelpers.UnexpectedTimeoutMilliseconds));
267+
Monitor.Exit(lockObj);
268+
});
269+
270+
backgroundTestDelegates.Add(lockObj =>
271+
{
272+
readyBarrier.SignalAndWait();
273+
Assert.True(
274+
Monitor.TryEnter(lockObj, TimeSpan.FromMilliseconds(ThreadTestHelpers.UnexpectedTimeoutMilliseconds)));
275+
Monitor.Exit(lockObj);
276+
});
277+
278+
backgroundTestDelegates.Add(lockObj =>
279+
{
280+
readyBarrier.SignalAndWait();
281+
bool lockTaken = false;
282+
Monitor.TryEnter(lockObj, ThreadTestHelpers.UnexpectedTimeoutMilliseconds, ref lockTaken);
283+
Assert.True(lockTaken);
284+
Monitor.Exit(lockObj);
285+
});
286+
287+
backgroundTestDelegates.Add(lockObj =>
288+
{
289+
readyBarrier.SignalAndWait();
290+
bool lockTaken = false;
291+
Monitor.TryEnter(
292+
lockObj,
293+
TimeSpan.FromMilliseconds(ThreadTestHelpers.UnexpectedTimeoutMilliseconds),
294+
ref lockTaken);
295+
Assert.True(lockTaken);
296+
Monitor.Exit(lockObj);
297+
});
298+
299+
int testCount = backgroundTestDelegates.Count * 2; // two iterations each, one for thin lock and one for aware lock
300+
readyBarrier = new Barrier(testCount + 1); // plus main thread
301+
var waitForThreadArray = new Action[testCount];
302+
for (int i = 0; i < backgroundTestDelegates.Count; ++i)
303+
{
304+
int icopy = i; // for use in delegates
305+
Thread t =
306+
ThreadTestHelpers.CreateGuardedThread(out waitForThreadArray[i * 2],
307+
() => backgroundTestDelegates[icopy](thinLock));
308+
t.IsBackground = true;
309+
t.Start();
310+
t = ThreadTestHelpers.CreateGuardedThread(out waitForThreadArray[i * 2 + 1],
311+
() => backgroundTestDelegates[icopy](awareLock));
312+
t.IsBackground = true;
313+
t.Start();
314+
}
315+
316+
lock (thinLock)
317+
{
318+
lock (awareLock)
319+
{
320+
readyBarrier.SignalAndWait(ThreadTestHelpers.UnexpectedTimeoutMilliseconds);
321+
Thread.Sleep(ThreadTestHelpers.ExpectedTimeoutMilliseconds);
322+
}
323+
}
324+
foreach (Action waitForThread in waitForThreadArray)
325+
waitForThread();
326+
}
327+
328+
// When the current thread has the lock, have background threads wait for the lock in various ways and time out
329+
// after a short duration
330+
{
331+
var backgroundTestDelegates = new List<Action<object>>();
332+
Barrier readyBarrier = null;
333+
334+
backgroundTestDelegates.Add(lockObj =>
335+
{
336+
readyBarrier.SignalAndWait();
337+
Assert.False(Monitor.TryEnter(lockObj, ThreadTestHelpers.ExpectedTimeoutMilliseconds));
338+
});
339+
340+
backgroundTestDelegates.Add(lockObj =>
341+
{
342+
readyBarrier.SignalAndWait();
343+
Assert.False(
344+
Monitor.TryEnter(lockObj, TimeSpan.FromMilliseconds(ThreadTestHelpers.ExpectedTimeoutMilliseconds)));
345+
});
346+
347+
backgroundTestDelegates.Add(lockObj =>
348+
{
349+
readyBarrier.SignalAndWait();
350+
bool lockTaken = false;
351+
Monitor.TryEnter(lockObj, ThreadTestHelpers.ExpectedTimeoutMilliseconds, ref lockTaken);
352+
Assert.False(lockTaken);
353+
});
354+
355+
backgroundTestDelegates.Add(lockObj =>
356+
{
357+
readyBarrier.SignalAndWait();
358+
bool lockTaken = false;
359+
Monitor.TryEnter(
360+
lockObj,
361+
TimeSpan.FromMilliseconds(ThreadTestHelpers.ExpectedTimeoutMilliseconds),
362+
ref lockTaken);
363+
Assert.False(lockTaken);
364+
});
365+
366+
int testCount = backgroundTestDelegates.Count * 2; // two iterations each, one for thin lock and one for aware lock
367+
readyBarrier = new Barrier(testCount + 1); // plus main thread
368+
var waitForThreadArray = new Action[testCount];
369+
for (int i = 0; i < backgroundTestDelegates.Count; ++i)
370+
{
371+
int icopy = i; // for use in delegates
372+
Thread t =
373+
ThreadTestHelpers.CreateGuardedThread(out waitForThreadArray[i * 2],
374+
() => backgroundTestDelegates[icopy](thinLock));
375+
t.IsBackground = true;
376+
t.Start();
377+
t = ThreadTestHelpers.CreateGuardedThread(out waitForThreadArray[i * 2 + 1],
378+
() => backgroundTestDelegates[icopy](awareLock));
379+
t.IsBackground = true;
380+
t.Start();
381+
}
382+
383+
lock (thinLock)
384+
{
385+
lock (awareLock)
386+
{
387+
readyBarrier.SignalAndWait(ThreadTestHelpers.UnexpectedTimeoutMilliseconds);
388+
foreach (Action waitForThread in waitForThreadArray)
389+
waitForThread();
390+
}
391+
}
392+
}
393+
}
394+
212395
[Fact]
213396
public static void Wait_Invalid()
214397
{
215398
var obj = new object();
216-
AssertExtensions.Throws<ArgumentNullException>("obj", () => Monitor.Wait(null));
217-
AssertExtensions.Throws<ArgumentNullException>("obj", () => Monitor.Wait(null, 1));
218-
AssertExtensions.Throws<ArgumentNullException>("obj", () => Monitor.Wait(null, TimeSpan.Zero));
219-
AssertExtensions.Throws<ArgumentOutOfRangeException>("millisecondsTimeout", () => Monitor.Wait(null, -1));
220-
AssertExtensions.Throws<ArgumentOutOfRangeException>("timeout", () => Monitor.Wait(null, TimeSpan.FromMilliseconds(-1)));
399+
Assert.Throws<ArgumentNullException>(() => Monitor.Wait(null));
400+
Assert.Throws<ArgumentNullException>(() => Monitor.Wait(null, 1));
401+
Assert.Throws<ArgumentNullException>(() => Monitor.Wait(null, TimeSpan.Zero));
402+
Assert.Throws<ArgumentOutOfRangeException>(() => Monitor.Wait(obj, -2));
403+
AssertExtensions.Throws<ArgumentOutOfRangeException>("timeout", () => Monitor.Wait(obj, TimeSpan.FromMilliseconds(-2)));
221404
}
222405

223406
[Fact]

0 commit comments

Comments
 (0)