rationale behind lock inside lock?

Question

I am reviewing an example code in a book and came across the following code(simplified). In the code, when Subscribe(T subscriber) is called, the thread enters into a lock section. and then, when code inside the lock calls AddToSubscribers(T subscriber) method, the method has another lock. why is this second lock necessary?

public abstract class SubscriptionManager<T> where T : class 
{
   private static List<T> subscribers;
   private static void AddToSubscribers(T subscriber)
   {
      lock (typeof(SubscriptionManager<T>))
      {
         if (subscribers.Contains(subscriber))
            return;
         subscribers.Add(subscriber);
      }
   }

   public void Subscribe(T subscriber)
   {
      lock (typeof(SubscriptionManager<T>))
      {
         AddToSubscribers(subscriber);
      }
   }
}

Answer 1

In that context, it isn't; however, since locks are re-entrant that can be useful to ensure that any other caller of AddToSubscribers observes the lock. Actually, for that reason I'd say "remove it from Subscribe and just let AddToSubscribers do the locking".

However! A lock on a Type is pretty dangerous. A field would be safer:

// assuming static is correct
private static readonly object syncLock = new object();

and lock(syncLock). Depending on when subscribers is assigned, you might also get away with lock(subscribers) (and no extra field).

I should also note that having an instance method add to static state is pretty.... unusual; IMO Subscribe should be a static method, since it has nothing to do with the current instance.

Answer 2

In the code you posted, it isn't necessary. But then again, the code you posted is incomplete - for example the subscribers list is never initialized.

Locking on typeof(SubscriptionManager) is probably not a good idea either - locking on the subscribers field would be better - but would require the subscribers field to be initialized, e.g.

private static List<T> subscribers = new List<T>();