How to make a static method thread safe?

Question

I wrote a static class, which is a repository of some functions that I call from another class.

public static class CommonStructures { public struct SendMailParameters { public string To { get; set; } public string From { get; set; } public string Subject { get; set; } public string Body { get; set; } public string Attachment { get; set; } } } public static class CommonFunctions { private static readonly object LockObj = new object(); public static bool SendMail(SendMailParameters sendMailParam) { lock (LockObj) { try { //send mail return true; } catch (Exception ex) { //some exception handling return false; } } } private static readonly object LockObjCommonFunction2 = new object(); public static int CommonFunction2(int i) { lock (LockObjCommonFunction2) { int returnValue = 0; try { //send operation return returnValue; } catch (Exception ex) { //some exception handling return returnValue; } } } } 

Question 1: For my second method, CommonFunction2, I use a new static lock, i.e. LockObjCommonFunction2 in this example, or can I reuse the same LockObj lock object defined at the beginning of the function.

Question 2: Is there anything that can lead to thread related problems, or can I improve the code to be a safe thread.

Quesiton 3: Could there be any problems when passing a generic class instead of struct .. in this example SendMailParameters (which I use to wrap all the parameters, instead of having multiple parameters for the SendMail function)?

Regards, MH

Answer 1

Question 1: For my second method, CommonFunction2, I use a new static lock, i.e. LockObjCommonFunction2 in this example, or I can reuse the same LockObj lock object defined at the beginning of the function.

If you want to synchronize these two methods, you need to use the same lock for them. For example, if thread1 gets access to your method 1, and thread2 gets access to your method2, and you want them to not be able to access the internal ones at the same time, use the same lock . But if you just want to restrict simultaneous access only to either method 1 or 2, use different locks .

Question 2: Is there anything that can lead to problems, or can I improve the code to be a safe thread.

Always remember that shared resources (for example, static variables, files) are not thread safe, since all threads are easily accessible to them, so you need to use any synchronization (through locks, signals, mutexes, etc.).

Quesiton 3: Could there be any problems when passing a generic class instead of struct .. in this example SendMailParameters (which I use when completing all the parameters, instead of having multiple parameters for the SendMail function)?

As long as you apply the correct synchronization, it will be thread safe. For structures, see this as a reference.

The bottom part is that you need to apply the correct synchronization operations to everything that is in shared memory. Also, you should always consider the area in which you create the thread, and the state of the variables used by each method. Do they change state or simply depend on the internal state of the variable? Does this thread always create an object, although it is static / shared? If so, then it should be thread safe. Otherwise, if it just reuses a specific share, then you must apply proper synchronization. And, more importantly, even without a common resource , deadlocks can still occur, so remember the basic rules in C # to avoid deadlocks . PS thanks to Euphoric for publishing an article by Eric Lippert.

But be careful with your syncs. To the extent possible, limit their scope only where the total resource changes. Because it can lead to uncomfortable bottlenecks for your application, where performance will be severely affected.

  static readonly object _lock = new object(); static SomeClass sc = new SomeClass(); static void workerMethod() { //assuming this method is called by multiple threads longProcessingMethod(); modifySharedResource(sc); } static void modifySharedResource(SomeClass sc) { //do something lock (_lock) { //where sc is modified } } static void longProcessingMethod() { //a long process } 

Answer 2

You can reuse the same lock object as many times as you want, but this means that none of the areas of the code surrounded by the same lock can be accessed simultaneously by multiple threads. Therefore, you need to plan accordingly and carefully.

Sometimes it’s better to use one lock object for multiple locations if there are several functions that edit the same array, for example. In other cases, more than one lock object is better, because even if one section of the code is locked, the other may still work.

Multithreaded coding is all about careful planning ...

To be super-duper safe, by potentially writing much slower code ... you can add an accessory to your static class surrounding the lock. Thus, you can make sure that none of the methods of this class will ever be called by two threads at the same time. This is a pretty brute force, and definitely a no-no for professionals. But if you are just familiar with how this works, this is not a bad place to start learning.

Answer 3

1) As for the first, it depends on what you want:

As it is (two separate locking objects) - none of the two threads will execute the same method at the same time, but they can simultaneously execute different methods.

If you go to one lock object, then two threads will not execute these partitions under the common lock object.

2) There is nothing in your snippet that would amaze me, but there is not much code. If your repository calls methods from itself, then you may have a problem, and there is a world of problems that you may encounter :)

3) As for the structures, I would not use them. Using classes is better / easier, since there is another bag of problems related to structures, you just don't need these problems.

Answer 4

The number of lock objects used depends on what data you are trying to protect. If you have multiple variables that are read / updated for multiple threads, you must use a separate lock object for each independent variable. Therefore, if you have 10 variables that form 6 independent groups of variables (as far as you intend to read and write them), you should use 6 lock objects for better performance. (An independent variable is one that is read / written to several threads without affecting the value of other variables. If 2 variables are to be read together for a given action, they are dependent on each other, so they should be locked together. Hope this is not too confusing.)

Locked areas should be kept as short as possible for maximum performance - every time you lock a code area, no other thread can enter that area until the lock is released. If you have several independent variables, but use too few lock objects, your performance will suffer because your blocked regions will grow longer.

Having more lock objects allows for higher parallelism, since each thread can read / write another independent variable - threads should wait from each other only if they try to read / write variables that are dependent on each other (and thus are blocked through the same lock object).

In your code, you should be careful with your SendMailParameters input parameter - if it is a reference type (class, not structure), you must make sure that its properties are locked or multiple threads are not accessible to them. If it is a reference type, it is just a pointer and without locking inside its getters / seters properties, several threads may try to read / write some properties of the same instance. If this happens, your SendMail() function may end up using a damaged instance. SendMail() lock inside SendMail() is not enough - you must also protect the properties and methods of SendMailParameters .