>Hi all,
>
>I have this method I am refactoring for various reasons. There is a lot of nesting and plan to remove some of it. I am wondering about this intital test inside of a lock statement and wonder if it would be safe to move it outside of the lock construct.
>
>This is what it looks like now.
>

>public void FindActiveWebService()
>{
>    lock (FindWebServiceSync)
>    {
>        if (!IsFindingActiveWebService)
>        {
>            IsFindingActiveWebService = true;
>            try
>            {
>                // Lots of crappy code and nested code in here with loops and such.
>                // Planning to refactor all this junk.
>            }
>            finally
>            {
>                IsFindingActiveWebService = false;
>            }
>        }
>    }
>}
>

>
>Wondering if it would be just as safe to do this and remove the extra if structure.
>

>public void FindActiveWebService()
>{
>    // Moved outside the lock
>    if (IsFindingActiveWebService)
>        return;
>
>    lock (FindWebServiceSync)
>    {
>        IsFindingActiveWebService = true;
>        try
>        {
>            // Lots of crappy code and nested code in here with loops and such.
>            // Planning to refactor all this junk.
>        }
>        finally
>        {
>            IsFindingActiveWebService = false;
>        }
>    }
>}
>

>
>Thanks for any thoughts.

Presumably, in the current code, IsFindingActiveWebService is referenced outside of the FindActiveWebService() method for some purpose - if it is not then it is redundant since no other thread would get inside the lock whilst it is set true.

I guess your proposed change would prevent a second thread attempting the lock maybe 99.999% of the time but there's always a chance the 'if' will execute before IsFindingActiveWebService gets set true. But when that happens it presumably would be no different to running the method consecutively with no lock contention ?