7

I'm a self taught C# programmer, I've missed some bits here and there when it comes to having a very thorough understanding about things, and now I've stumbled across something I haven't been able to find an answer to on SO. I'm trying to get a better understanding about thread safety in C#, but let me first specify the context.

I'm currently developing a Windows service which goes off and does some monitoring work based on a schedule which resides in a SQL Server database. It is going to monitor some servers by making http requests to a number of "client servers", a client installed on those servers will respond with the requested information.

As this monitor service might get quite busy, I have set it up to stick every "scheduled instruction" in a new thread when it is scheduled to do the work. This is to make sure my timer keeps ticking along nicely, ready to fire off the next instruction to the next "client server".

A part of each instruction is that is has to log in the database that it has executed successfully and what the response was and so on. Now I have in my monitor service a public static class Logger, I believe this is handy as I can now easily call it this way Logger.Log(... ) whenever I need to log things. This logging happens in this class through EF into the SQL Server database.

To me this all sounds really cool, and I'm quite happy with how it all works, but I haven't load tested anything as of yet. The problem I have with all of this is that my brain tells me that since my logger class is static -and according to my understanding therefore it is only instantiated once?- if more than 1 thread tries to call Logger.Log(.. ) at the exact same time, bad things will happen to my monitor service.

Is there someone here who can enlighten me? Is my thinking right or wrong? And if you know the answer, please explain it clearly because I would love to understand it. :)

Update:

Thanks for responses up till now, things are getting clearer, as people are asking more details about the Log method, and I'm not at my development PC at the moment, I will try to explain the way it works in a bit more detail.

All the Log method does is add a record to the SQL database through EF based on data from some previously instantiated objects which are passed in to the method as parameters. The database context is instantiated as a static private variable on the static class. The reason for this is so that I don't have to keep putting using statements in my overloads.

Mathijs Flietstra
  • 12,900
  • 3
  • 38
  • 67
  • 2
    It depends on what your `Log` method is doing. Post the code so we can analyze it. – zimdanen Oct 14 '13 at 19:11
  • 2
    @patxy That's nonsense. Considering how popular web applications are, it'd be ridiculous for database drivers to not be threadsafe. (You generally perform different statements on new or pooled connections anyway.) And a static method wouldn't change anything w/r/t threading and thread safety. – millimoose Oct 14 '13 at 19:23
  • @millimoose, I'd give you +10 rep for the "with respect to" (w/r/t) abbreviation. Ha, or at least I **think** that's what it means. – Mike Perrenoud Oct 14 '13 at 19:26
  • 2
    he's just playing a lot of fantasy football. Wide Receiver/Runningback/Tight End – Jonesopolis Oct 14 '13 at 19:27
  • 1
    @Jonesy Those sound like umm... let's go with "euphemisms". – millimoose Oct 14 '13 at 19:35

3 Answers3

3

static provides the opportunity for dangerous code, but it does not guarantee it. If you're using a static class/method, you have to be careful not to use any instance data.

What does that mean in your case? Basically, you want to instantiate your DbContext within the Log method, do your logging, and Dispose the DbContext (wrap the usage in a using statement). As long as there's no sharing of instance data, you'll be fine.

However, if you're doing something in the static constructor or using class-level variables, you could be creating issues.

Edit: In your specific case, you should not be sharing the DbContext across all of your threads. Take a look here for a discussion of the correct scope for a DbContext. It should be instantiated in each method.

This blog entry states the following (and provides explanations):

Most of [these considerations] tend to point towards a short lived context that isn’t shared.

So that is my recommended rule of thumb.

Community
  • 1
  • 1
zimdanen
  • 5,508
  • 7
  • 44
  • 89
  • "There isn't really enough information to answer the question." Then why did you answer it? – Servy Oct 14 '13 at 19:34
  • @Servy: I answered it to give an explanation of what is and isn't thread-safe, which is *in general* what he was asking. However, the specific question ("Is this method safe?") was unanswerable until he provided his update, at which point I also responded to that. – zimdanen Oct 14 '13 at 19:40
  • +1, thanks for your answer and helping to clear up my understanding, I will definitely read up on the info you provided on how I should do things with regards to the `DBContext`, but another answer gave me the eureka moment I needed to clearly understand why a static class can be called twice at the exact same time. – Mathijs Flietstra Oct 16 '13 at 07:59
3

Each method, no matter static or virtual, will have its own frame, so there is no thread problem involved. The problem occurs in the method implementation: some static methods will use static variables or static resources, and they are all the same pipe, and you will run into race conditions. But local variables declared inside a static method aren't static, so if your method does not modify static variables or resources, you will be just fine.

lolol
  • 4,287
  • 3
  • 35
  • 54
  • @MathijsFlietstra I just saw your update. Wrap your database variable inside a using statement and your code will be fine. If you wrap your connection inside a using the database will deal with each method frame by itself (the windows log does it too, but a txt file or mdb file don't, that is the real concern). – lolol Oct 14 '13 at 19:41
  • Thanks, I'm probably not the brightest as I still didn't really get it after reading 3 good answers, but you get the tick as your use of the word "frame" led me to do a Google search on frames in programming which led me to the word "call stack", which in turn fixed up my misunderstanding. I understood that a static class was "instantiated" once inside the main thread, and then would be called from another thread, it would therefore give problems if called at the exact same time by 2 threads. I now believe this is the wrong understanding, the static class is not "instantiated" in he main.. – Mathijs Flietstra Oct 16 '13 at 07:44
  • 2
    ..thread, it just sits "un-instantiated" in the main thread with methods which are accessible from any other thread. Now when a thread needs to call the method, it knows where to look for it and sticks it on its own "call stack", and since every thread has its own "call stack", every thread can call the method on its own "call stack" at the very same time. And that's why there will only be a problem if I access static variables which live outside the method, as they are not on the threads "call stack" but only exist in 1 spot in memory. Thanks for your answer, please correct me if I'm wrong. – Mathijs Flietstra Oct 16 '13 at 07:52
  • @MathijsFlietstra You got it, that is exactly it, each method, no matter static or not, will have its own stack. The problem is not about a method being static or not, but about what you do inside the method. Because static member variables races or non cleanable resources left behind (like a non disposed connection). A static method stays in memory, any dirty left behind will stay there. – lolol Oct 16 '13 at 12:58
3

What does the documentation for your Logger class say with respect to thread safety? There is nothing inherently thread-unsafe about a static class or method.

If the method or property you invoke, whether it's static or not

  • only references veriables local to the method in question (e.g. doesn't reference any instance or static members), and
  • creates its own instances of any other classes with which it needs to collaborate

You should be thread-safe. Note that any methods or properties invoked in other classes must likewise be thread-safe.

Nicholas Carey
  • 71,308
  • 16
  • 93
  • 135
  • +1 and thanks for your answer, it provides very useful information, but another answer provided the trigger I needed to fix up my misunderstanding. – Mathijs Flietstra Oct 16 '13 at 08:04