Why expression is always true for 'double-checked-locking'?

Question

I have singleton object 'Service' and two methods to initialize and release it:

public class BaseService
{
    protected static readonly object StaticLockObject = new object();
}

public abstract class WebServiceBase<TService> : BaseService
    where TService : System.Web.Services.Protocols.SoapHttpClientProtocol, new()
{
    protected static void EnsureServiceIsOpened()
    {
        if (Service == null)
        {
            lock (StaticLockObject)
            {
                if (Service == null)
                {
                    Service = new TService();
                }
            }
        }
    }

    protected static void EnsureServiceIsClosed()
    {
        if (Service != null)
        {
            lock (StaticLockObject)
            {
                if (Service != null) // Why expression is always true
                {
                    Service.Dispose();
                    Service = null;
                }
            }
        }
    }

For the line with comment resharper (I use version 5.1) displays a mentioned warning...

Question 1: Why?

Question 2: Why it doesn't display "similar" message in 'EnsureServiceIsOpened' method?

Thanks.

Answer 1

Resharper does a limited analysis of the code, and sees the two nested if statements checking exactly the same. In a single threaded environment, the comment by resharper holds perfectly true - there is no way Service will be null after the first if. In a multithreaded envrionment of course, this doesn't apply, as Service could be changed from the outside. This is a case where you should annotate the code for resharper or surpress the message for this file, as it doesn't hold true in a multithreaded environment.

Answer 2

It appears that Resharper is making a mistake in this analysis. Since Service is not a local variable, it cannot know that that expression will always be true.

The reason why the error doesn't show up for the first version is likely because the double-checked-lock idiom is so common, and that is its usual form. They probably tested for that case and removed the erroneous warning.

The technique displayed in EnsureServiceIsClosed is not common because it contains a race condition. Another thread could be using the object indicated by Service while—or after—it is being disposed. This would not be possible if the code that uses the Service locks on StaticLockObject, but if it does take the lock, then there is no reason to do all this double-checked locking rigamarole around creating and disposing the object. Therefore, it is virutally certain that this code is flawed.