Do I need to lock "read only" services when using threading?

Question

I have a service that I am rewriting to use threading. I understand that state from one thread should not be accessed by another, but I'm a little confused by what constitutes 'state'. Does that mean any field/property/method outside of the method scope?

Specifically, my service looks something like this:

public class MyService
{
      private IRepository<MyClass> repository;
      private ILogger log;
      ...
      public void MyMethod()
      {
        ...
        var t = new Thread(MyMethodAsync);
        t.Start(someState);
      }

      //Is this OK???
      public void MyMethodAsync(object state)
      {
          var someState = (MyState)state;
          log.Log("Starting");
          var someData = repository.GetSomeData(someState.Property);
          //process data
          log.Log("Done");            
      }

      //Or should I be doing this:
      public void MyMethodAsync2(object state)
      {
          var someState = (MyState)state;
          lock(log){
             log.Log("Starting");  }

          lock(repository){         
             var someData = repository.GetSomeData(someState.Property);}

          //process data   
          lock(log){
             log.Log("Done"); }            
      }
}

Answer 1

"State" is all the data contained in the class, and the real issue as far as concurrency goes is write access, so your intuition is right.

Answer 2

Er...nope, you don't need to lock resources that are read-only. The purpose of locking them is so that if you need to check the value of a resource before writing it then another resource can't change the value between your read and your write. i.e.:

SyncLock MyQueue
  If MyQueue.Length = 0 Then
    PauseFlag.Reset
  End If
End SyncLock

If we were to check the length of our queue before we set the flag to pause our process queue thread and then another resource were to add an item to the queue, then our process queue thread would sit in a paused state while an item potentially could've been added between checking the queue length and setting the pause flag...

If all resources are only accessing the queue in a read only fashion (not that I could think of a single useful application of a read-only queue) then there's no need to lock it.

Answer 3

Even worse, locking read-only structures is a good way to create deadlocks.