Why is the expression always true for "double check-lock"?

Question

I have a singleton "Service" object and two methods for initializing and releasing 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 the comment resharper (I am using version 5.1) the indicated warning is displayed ...

Question 1: Why?

Question 2: Why doesn’t it display a "similar" message in the "EnsureServiceIsOpened" method?

Thanks.

Answer 1

This is a bug in the ReSharper 5.X code analysis program. Fixed in ReSharper 6.0.

BTW, ReSharper 6 brings more data to analyze double locks :)

Answer 2

Resharper does a limited code analysis and sees that two nested if check the same thing. In one streaming environment, the resharper comment is right - no Service path will be null after the first if . Of course, in a multi-threaded environment, this does not apply, since Service could be changed externally. This is the case when you must annotate the code for resharper or suppress the message for this file, as it does not run in a multi-threaded environment.

Answer 3

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

The reason why the error does not appear for the first version is probable, because the idiom with double check for blocking is so common, and this is its usual form. They probably tried this case and removed the erroneous warning.

The technique displayed in EnsureServiceIsClosed is not general because it contains a race condition. Another thread may use the object designated by Service , while or after its removal. This would not be possible if the code that uses the service lock on StaticLockObject , but if it accepts the lock, then there is no reason to do all this double-checked rigamarole locking around creating and deleting an object. Therefore, it is obvious that this code is erroneous.