Lock() in multithreading program

Question

I have a simple program that simulates my error situation. I have a singleton class that gets a messages from several threads. The execution must be blocked until the function is executed.

class Program { private static TestClass test; static void Main(string[] args) { Thread a = new Thread(TestFunctionB); a.Start(); Thread b = new Thread(TestFunctionB); b.Start(); } private static void TestFunctionB() { TestClass test = TestClass.Instance; for (int i = 0; i < 15; i++) { test.Handle(i, Thread.CurrentThread.ManagedThreadId); } } } class TestClass { private readonly object _lockObject; private static TestClass _instance; private TestClass() { _lockObject = new object(); } public static TestClass Instance { get { return _instance ?? (_instance = new TestClass()); } } private void RunLocked(Action action) { lock (_lockObject) { action.Invoke(); } } public void Handle(int counter, int threadId) { Console.WriteLine("\nThreadId = {0}, counter = {1}\n", threadId, counter); RunLocked(() => { Console.WriteLine("\nFunction Handle ThreadId = {0}, counter = {1}\n", threadId, counter); for (int i = 0; i < 30; i++) { Console.WriteLine("Funktion Handle threadId = {0}, counter = {1}, i = {2}", threadId, counter, i); //Thread.Sleep(100); } }); Console.WriteLine("\nFunction Handle free ThreadId = {0}, counter = {1}\n", threadId, counter); } }

`

I excpect that threads write the output one after another, but in the console the threads outputs are mixed. Is the lock statement not correct?

What do you expect to happens? lock ensures what for duration of delegate no other thread will obtain it (but doesn't guarantee anything before or after it). So first thread will do complete cycle (30 iterations), then second. What do you want? To print single message? Exactly one from one thread and one from another? — Sinatr
– Sinatr, Commented Jul 25, 2016 at 14:51
I expect the same. But in console window i have f.e. 20 string from thread 1, then 12 strings from thread 2. — kyy8080
– kyy8080, Commented Jul 25, 2016 at 14:56
The reason could be indeed poorly implemented singleton, see @Scott answer. Initially I though you get this behavior, but you expect something different. — Sinatr
– Sinatr, Commented Jul 25, 2016 at 14:58

Scott Chamberlain · Accepted Answer · 2016-07-26 13:34:53Z

I don't know if it is your only problem but get { return _instance ?? (_instance = new TestClass()); } is not atomic, you may end up with more than one instance returned.

Use the Lazy<T> class to guarantee that only one instance of the singleton is created.

class TestClass { private readonly object _lockObject; private readonly static Lazy<TestClass> _instance = new Lazy<TestClass>(x=> new TestClass()); private TestClass() { _lockObject = new object(); } public static TestClass Instance { get { return _instance.Value; } } ... }

If you don't have access to .NET 4.0 or newer you will need to lock around your singleton creation.

class TestClass { private readonly object _lockObject; private static readonly object _singletonLock = new Object(); private static TestClass _instance; private TestClass() { _lockObject = new object(); } public static TestClass Instance { get { if(_instance == null) { lock(_singletonLock) { if(_instance == null) { _instance = new TestClass (); } } } return _instance; } } ... }

Collectives™ on Stack Overflow

Lock() in multithreading program

1 Answer 1

Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Linked

Related