Thread Safety and locking objects

[zachattack05]

Quick question:

Is this valid, or am I missing a potential deadlock in the future:

static readonly object _LockThis = new object();


        private static bool _Status;


        private bool Status
        {
            get { lock (_LockThis) { return _Status; } }
            set { lock (_LockThis) { _Status = value; } }
        }

        private void Work()
        {
                while (Status)
                {
                    //do some work
                }
        }

        private void StopWorking()
        {
                Status = false;
        }

        private void StartWorking()
        {
                Status = true;
                Thread WorkThread = new Thread(Work);
                WorkThread.Start();
                StopWorking();
        }

[pseudorandom21]

I do that all the time, except using a Mutex. I'm extremely new to C# and only today learned of the "lock" statement.

From what I've read about multi-threading in C# you may not want your _Status to be static. I searched google again and couldn't find the exact page on it. Remember I'm still new to this C# business. The readonly on the _LockThis puzzles me also.

[minitech]

No, with the code as-is you have no potential deadlock problems. Static objects are fine, as long as you have a good reason to use them in the particular class. But as for locking static objects and using synchronization on static properties, go ahead. In this case, however, you might want to consider using alternative ways of stopping and starting operations on your thread using more advanced methods that were meant for precisely what you appear to want to do.

Depending on the situation, you might even be able to use Thread.Start() and Thread.Abort() and not even bother with variables in the first place. You just have to remember to use Abort() correctly.

[zachattack05]

minitech,

Thanks for the insight. It's for a socket server listening loop. Closing the socket will end the listening thread correctly, so I'll just do that, but it just got me thinking of ways to do it gracefully, never used it before, but was curious about it if I ever needed to use it.

[Momerath]

You wouldn't want to use a static lock with a non-static object. Remember that all objects of that type will be locked when one thread tries to access them.

[zachattack05]

You wouldn't want to use a static lock with a non-static object. Remember that all objects of that type will be locked when one thread tries to access them.

Wait a second. What?

I must have missed something. Objects of what type will be locked? Are they locked automatically? If that's true...oh my, the time I've spent agonizing over thread safety for objects that may, or may not need to be locked!

[Momerath]

In the example you gave above, you used a static object as the lock. That means any objects of that class share the same lock, even though they have their own instance variables. You also use a static Status variable, which means if you started 10 threads, changing the status would stop all the threads at once. Not sure if that is what you intended.

If it is what you intended, in production code I'd make sure that I commented that area to explain that :)

If it wasn't what you intended then you wouldn't need to lock the status as there is no way to deadlock it even if you are trying to stop it while the thread is checking the value. At most you'd do a couple more iterations of the while loop (as long as you made the status not static, that is)

[zachattack05]

Ahhhhhhh

I see what you are saying now! And yes, that is the intention, if the Status field holds a bool value indicating "true" if the server is "running," setting it to "false" would correctly break all of the threads in a "connection loop" out, so they can close the socket and end gracefully. Aborting the thread is dirty, and bleh...I don't even want to think about it.

[pseudorandom21]

I think the reason to not make it static is so that if creating more than one instance of the class to still enable proper multi-threading for all instances. For some other reason I can't remember I thought it shouldn't be static. If I remember, I'll let you know.

[zachattack05]

That's true. However, in this sitution, setting that field value to "false" is supposed to affect all instances of the class, and all associated threads.