Search code examples
c#loggingconcurrency

Using locks and logs in C#


Lately I noticed a strange warning in the Resharper. When I use a lock and call to the logger (in my case log4net), I receive a warning that the data member is not always used inside lock / it is not thread safe (because there are some places that I print to log outside the lock).

Obviously, I don't care about the warning because the logger should not be thread safe and can be called from many threads. My question is: Can I just ignore the warning or should I handle it?

As far as I understand, the logger can be used from many threads in the described situation because I locked the method and not the object. Am I right? Or should I create two ILogger data members?

In the following example we can see the usage of the Logger in two different methods without synchronization.

private class Test{
   private static readonly ILogger Logger = LoggerFactory.GetLogger(typeof(Test));

 void MethodWithLock()
 {
    lock(_padlock)
    {
      //some logic here
      Logger.Debug("dummy log");
      //some logic here
    }
 }

 void MethodWithoutLock()
 {
    //some logic here
    Logger.Debug("dummy log");
 }

Solution

  • True, loggers generally are thread-safe, so this warning is superfluous and can be ignored. ReSharper is just trying to be smart, but in this case it is just showing that it is not really that smart. ReSharper is assuming that every single member variable that you use inside the lock() clause must be a guarded variable, (a variable guarded by that lock() clause,) so it analyzes other uses of the variable and finds discrepancies. ReSharper simply does not know that Logger is just not a guarded variable.

    The easiest solution to your problem at hand, so that you can proceed with your life without spending too much time on this minor issue, is to simply replace the call to Logger with a call to a private method of this, which contains the actual call to Logger.

    This way, ReSharper will stop worrying about the use of Logger inside the lock() clause.