How to ensure a static class and its members are thread safe or not?

Question

0.00/5 (No votes)

See more:

C#

//Controller
    public ActionResult AddMail(Guid userId)
    {
        EmailNotification.QueueMailAync(userId);
        .
        .
        //Some stuffs - QueueMailAync may call more time in a request.
        EmailNotification.QueueMailAync(userId);
        .
        .
    }

    //BL
    public static class EmailNotification
    {
    public static void QueueMailAync(Guid userId)
    {
        HostingEnvironment.QueueBackgroundWorkItem
        (
            cancellationToken =>
            {
                try
                {
                    QueueMail(userId);
                }
                catch (Exception e)
                {
                    logger.Error(e);
                }
            }
        );
    }

    static void QueueMail(Guid userId)
    {
        using (var context = new DBEntities())
        {
            var user = context.Users.FirstOrDefault(u => u.Id == userId);

            string body = context.EmailContents.FirstOrDefault().BodyText;
            body = body.Replace("{USER_NAME}",user.UserName);

            var mail = new ArchOver.Model.Email();
            mail.BodyText = body;
            mail.UserId = userId;
            mail.Subject = "Aync";
            mail.EmailTo = user.Person.Email;

            context.Emails.Add(mail);
            context.SaveChanges();
        }
    }
}

What I have tried:

I'm new to parallel programming concepts. I'm trying to implement fire-and-forgot kind of method in my project. Somebody please evaluate the above code & let me whether the above is thread safe or not.

Thanks in Advance.

Posted 16-Mar-16 22:45pm

Balaganesan Ravichandran

Updated 17-Mar-16 1:41am

v3

Add a Solution

Comments

F-ES Sitecore 17-Mar-16 6:26am

As long as your database allows multiple records in the User table with the same userid (ie it's not a key or subject to a unique constraint) you should be ok. In terms of the code it looks thread safe, but obviously there is a chance that your QueueMail can run more than once for the same user so the crux of the matter is if that is ok or not, which depends on your database schema.

Balaganesan Ravichandran 17-Mar-16 6:33am

Yes, database allowing multiple records & QueueMailAync method will be called multiple times in the same request. It is just simple form of actual scenario. I will update my question for better understanding. Thanks for your reply.

Sascha Lefèvre 17-Mar-16 7:35am

Isn't this line a bug/typo?:
context.Users.Add(mail);

Balaganesan Ravichandran 17-Mar-16 7:42am

Good catch sascha. It is typo error. I have corrected the same. Thanks for the reply.

Sascha Lefèvre 17-Mar-16 10:33am

Did you downvote my answer? If so, I'd be interested to know why?

Balaganesan Ravichandran 18-Mar-16 5:59am

No Sascha. I'm not downvote your answer.

Sascha Lefèvre 18-Mar-16 6:10am

Alright - thank you for the reply :)

3 solutions

Solution 2

You might need to use locking, here is an example:

C#

private static readonly object SyncObject = new object(); 
 
public static void Log(string logMessage, TextWriter w)    { 
   // only one thread can own this lock, so other threads 
   // entering this method will wait here until lock is available. 
   lock(SyncObject) { 
      w.WriteLine("{0} {1}", DateTime.Now.ToLongTimeString(), 
          DateTime.Now.ToLongDateString()); 
      w.WriteLine("  :"); 
      w.WriteLine("  :{0}", logMessage); 
      w.WriteLine("-------------------------------"); 
      // Update the underlying file. 
      w.Flush(); 
   } 
}

Posted 16-Mar-16 23:14pm

RickZeeland

Comments

Sascha Lefèvre 17-Mar-16 6:05am

The inquirer is asking whether the above is thread safe or not. Saying he might need to use locking is no answer to that.

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Andy Lanng · Accepted Answer · 2016-03-16T23:11:00

Solution 1

It depends on how you use it.

UPDATE: This next paragraph is wrong. It is a static class. I'll leave it here for posperity
Here, you don't have a static class, you have a static instance of a class. The difference is that you still need to initialize it. The best way is to use "Lazy Initialization[^]". This is a shorthand way of thread-safe initialization.

If the class is a "stuct" then you don't have to worry too much about access to the parameters, but in either case it's better to be safe than sorry.

A "Lock" encapsulation prevents more than one thread entering the code at any one time.

The Lock is written like so:

UPDATE: Locks are used to avoid threads getting in each others way. The below example is an example only of how locks work.

C#

//The "key" object.  It can be any object but any lock with the same key can only be entered by one thread at a time.
var lockObj= new object();

public static int _prop;
public static int prop{
  get{
    Lock(lockObj){
      return _prop;
    }
  }
  set{
    Lock(lockObj){
      _prop = value;
    }
  }
}

The getter and setter both have locks with the same key. If a thread enters either one then any other thread will have to wait for the first thread to exit the lock before it can enter.

That's the basics of manual thread-safety. There are other things to keep in mind such as Thread Join, Wait etc. A google search should highlight the nuances.

Hope that helps ^_^
Andy

Posted 16-Mar-16 23:11pm

Andy Lanng

Updated 17-Mar-16 0:12am

v4

Comments

Balaganesan Ravichandran 17-Mar-16 5:36am

Thanks for the answer Andy Lanng. I'm having the below two query.

#1:
As said in the description, I wanted to send email from separate thread in the background. If I use lock, will it affect the main thread?

#2:
I got to know from google that, static methods with non reference type params can be thread safe. In the example, I have used only value type params. I know, EF entities are not thread safe. So I'm instantiating by using for each call. eager to know, where the race conditions is possible in my code?

Andy Lanng 17-Mar-16 5:51am

#1: Valid question, but it's safe: Lock will not stop any thread that doesn't encounter the lock. When you step through the code with threading (which is really confusing), only the threads that hit the locked code will wait. All other threads (including the main thread) will continue.
#2: A little more complex: EF interacts with the database. You should always use fresh connections (new dbcontext()) in each thread, or better yet for each query. EF will manage the connection pool for you (because there may be a limit, but you don't have to worry). The issue is that when one thread may read data, another updates it so the data in the first thread is out of date. Worse is when you get a deadlock on the db when two threads try to lock the same records. That's where Transactions come in. It is how the Database handles "threading" (multiple connections). That's a whole different kettle of fish!

Balaganesan Ravichandran 17-Mar-16 6:23am

Thanks for the reply on my childish question Andy Lanng.

Andy Lanng 17-Mar-16 7:27am

Not at all childish. Very valid!

Don't forget to accept the solution :P

Sascha Lefèvre 17-Mar-16 7:42am

I don't think it is a solution because it's not answering his question whether his methods are thread safe or not..

Balaganesan Ravichandran 17-Mar-16 8:26am

You have added one more solution to current problem. Probably safest. We can wrap all code blocks in QueueMail method inside a lock. This still be a fire and forget method & no need to worry about thread safety. But I want to know, did I left holes in my code for race condition? Is there any room for improvement? Upvoted ;)

Sascha Lefèvre 17-Mar-16 6:01am

It's not a static instance of a class, it's a static class.

Andy Lanng 17-Mar-16 6:04am

Ha - Your correct! This is what happens when I go on CodeProject before my first Coffee. I'll edit ^_^

Sascha Lefèvre 17-Mar-16 6:04am

Why would you use a lock here?

Andy Lanng 17-Mar-16 6:07am

No reason whatsoever, but my solution was "what is a lock", not "where should locks be used". I'll clarify in an edit

Sascha Lefèvre · Accepted Answer · 2016-03-17T01:41:00

Thread safety is a potential issue whenever threads may access the same resources simultaneously (where a resource can be as simple as a variable).

I would assume your shown code is thread safe. The resources that could be used simultaneously by multiple threads are your database and the logging-output. You create a local DbContext and read only once from the database and then write once, but writing a new entity and not updating an existing one, so there can't be issues with database locks. And the logging code should be thread safe by design.

So there shouldn't be any locks required.

How to ensure a static class and its members are thread safe or not?

3 solutions

Solution 1

Solution 3

Solution 2

Add your solution here

Preview 0

Existing Members

...or Join us