Why does this WCF Proxy code work?

Question

While debugging in an app, I found the following code. It's obviously wrong, but for some strange reason it works, and I can't seem to understand why. It would seem to me that the Proxy would be disposed as soon as it was created, yet calling methods on this proxy work correctly to connect to the WCF service.

Can anyone explain why this code doesn't blow up?

private static IMyService _proxy = null;
private static IMyService Proxy
{
  get
  {
    if (_proxy == null)
    {
      using (_proxy as IDisposable)
      {
        ChannelFactory<IMyService> factory = 
             new ChannelFactory<IMyService>("MyService");
        _proxy = factory.CreateChannel();
      }
    }
    return _proxy;
  }
}

@DanielA.White - That's what I was thinking.. perhaps the As creates a copy of the reference (which in this case is null anyways?) — Erik Funkenbusch, Jun 25 '14 at 15:43
@FrédéricHamidi - That was another theory I had... Can anyone support this? (ildism maybe?) — Erik Funkenbusch, Jun 25 '14 at 15:46
[Dispose only called on non-null objects](http://stackoverflow.com/questions/2522822/c-sharp-using-statement-with-a-null-object)? — StuartLC, Jun 25 '14 at 15:46
There is more: the [documentation](http://msdn.microsoft.com/en-us/library/yh598w02.aspx) actually says `within the using block, the object is read-only and cannot be modified or reassigned.` The compiler should not let the assignment pass, and I have no idea why it does. *Oh yes, I have, it's not the "same" object. `foo as IDisposable` is not the same thing as `foo`.* — Frédéric Hamidi, Jun 25 '14 at 15:48
@StuartLC - Yes, I'm aware of that.. but the variable is not null when the method ends. The question is, is this because a null check is done before the scope is executed only? — Erik Funkenbusch, Jun 25 '14 at 15:49
@FrédéricHamidi - probably because the object is declared outside of the using block. — Erik Funkenbusch, Jun 25 '14 at 15:50
@AK_ - This code is quite old, and has been in the code base since at least VS2008, we're currently using VS2013, but targeting .net 4. — Erik Funkenbusch, Jun 25 '14 at 15:54
@FrédéricHamidi the compiler clearly creates a copy in the `using(...)` — AK_, Jun 25 '14 at 16:03

score 6 · Accepted Answer · edited May 23 '17 at 12:05

This really is interesting!.

It is apparent that combination of the use of an existing variable in the using() statement and the reassignment of the original within the block, results in a new reference which is passed to Dispose(), and not the existing _proxy, as per the IL below.

In fact, LinqPad issues a warning to this effect:

Possibly incorrect assignment to local 'variable' which is the argument to a using or lock statement. The Dispose call or unlocking will happen on the original value of the local.

However this behaviour does not appear to require a cast to as IDisposable, nor does the static seem to have any effect.

Disassembling the following code to IL in LINQPad:

IMyService _proxy = null;
if (_proxy == null)
{
   using (_proxy)
   {
      _proxy = new SomeService();
   }
}

Yields

IL_0001:  ldnull      
IL_0002:  stloc.0     // _proxy
IL_0003:  ldloc.0     // _proxy
IL_0004:  ldnull      
IL_0005:  ceq         
IL_0007:  ldc.i4.0    
IL_0008:  ceq         
IL_000A:  stloc.1     // CS$4$0000
IL_000B:  ldloc.1     // CS$4$0000
IL_000C:  brtrue.s    IL_002D
IL_000E:  nop         
** IL_000F:  ldloc.0     // _proxy
** IL_0010:  stloc.2     // CS$3$0001
IL_0011:  nop         
IL_0012:  newobj      UserQuery+SomeService..ctor
IL_0017:  stloc.0     // _proxy
IL_0018:  nop         
IL_0019:  leave.s     IL_002B
$IL_001B:  ldloc.2     // CS$3$0001
$IL_001C:  ldnull      
$IL_001D:  ceq         
IL_001F:  stloc.1     // CS$4$0000
IL_0020:  ldloc.1     // CS$4$0000
$IL_0021:  brtrue.s    IL_002A
** IL_0023:  ldloc.2     // CS$3$0001
IL_0024:  callvirt    System.IDisposable.Dispose
IL_0029:  nop         
IL_002A:  endfinally  
IL_002B:  nop

As can be seen (by the **'s), that a new loc 2 is created, and it is this reference which is passed to Dispose(). But prior to this is a check for null ($), which bypasses the Dispose in any event.

Contrast this with the explicit try-finally:

 if (_proxy == null)
 {
    try
    {
       _proxy = new SomeService();
    }
    finally
    {
       _proxy.Dispose();
    }
 }

IL_0001:  ldnull      
IL_0002:  stloc.0     // _proxy
IL_0003:  ldloc.0     // _proxy
IL_0004:  ldnull      
IL_0005:  ceq         
IL_0007:  ldc.i4.0    
IL_0008:  ceq         
IL_000A:  stloc.1     // CS$4$0000
IL_000B:  ldloc.1     // CS$4$0000
IL_000C:  brtrue.s    IL_0025
IL_000E:  nop         
IL_000F:  nop         
IL_0010:  newobj      UserQuery+SomeService..ctor
IL_0015:  stloc.0     // _proxy
IL_0016:  nop         
IL_0017:  leave.s     IL_0023
IL_0019:  nop         
** IL_001A:  ldloc.0     // _proxy
IL_001B:  callvirt    System.IDisposable.Dispose
IL_0020:  nop         
IL_0021:  nop         
IL_0022:  endfinally  
IL_0023:  nop

Where it can be clearly seen that it is the "original" _proxy which is Disposed, without the extra null check done in the using(), confirming the docs.

Needless to say this code is evil:

_proxy will never be Disposed, viz the using is redundant.
It relies on passing null to using not failing.
Because _proxy is defined prior to the using, it can be reassigned within the block, unlike variables declared IN the using statement - e.g. using (var _proxy = ...) would be read-only, and attempting to mutate such a variable would result in a compile time error anyway.
The merits of static singleton channels aside, the lazy initialization of _proxy is also not thread-safe (e.g. no double-check lock), and could be simplified with a private static readonly Lazy<IMyService>(() => ...) and a simple getter.

Thanks for the disassembly and analysis. It seems clear that the `as IDisposable` is creating a new reference, and in fact removing it results in a compiler error. I'm guessing the `as IDisposable` was added to make it compile. Yes, the code is evil, and I am rewriting it. But, I always like to understand why code functions the way it does. — Erik Funkenbusch, Jun 26 '14 at 04:17
The `_proxy As IDisposable` is probably just needed because `IMyService` doesn't implement `IDisposable` whereas another interface implemented by `Proxy` does, hence the compiler error. However, in the above, if `IMyService` directly implements `IDisposable`, the additional IL var is still created for disposal even without a cast. Thanks for sharing this code - it almost looks right at first glance - great for interviewee questions ;). I'm guessing that the compiler ideally should have `temporarily` made `_proxy` readonly in the using `block`, but can't support this. — StuartLC, Jun 26 '14 at 05:08

Frédéric Hamidi · Answer 2 · 2014-06-25T22:15:33.103

2

That code works because of:

using (_proxy as IDisposable)

The expression _proxy as IDisposable is not _proxy, and that explains why _proxy can be assigned to within the using block without the C# compiler complaining in any way.

In addition, even if _proxy is subsequently set to a non-null disposable instance, the original value as "seen" by the using block (_proxy as IDisposable) will still be null (there are no references involved in this context).

Therefore, nothing is disposed of, a new proxy is created and cached, and that code succeeds.

I'm still wondering about the potential reasons why one would like to write such a using statement, however.

edited Jun 25 '14 at 22:15

answered Jun 25 '14 at 15:54

Frédéric Hamidi

258,201
41
486
479

Do you have any references to the fact that casting a variable creates a new reference? – Erik Funkenbusch Jun 25 '14 at 15:57
This question mentions reference conversions point to the same reference, but user-defined conversions do not, the question is, what type is this? http://stackoverflow.com/questions/1064297/does-casting-an-object-in-c-sharp-always-return-a-reference-to-the-intial-object – Erik Funkenbusch Jun 25 '14 at 16:00
2

@Erik, that actually a matter of value, not reference. `_proxy as IDisposable` returns the *value* `null`, and that value will remain `null` even if `_proxy` subsequently changes. – Frédéric Hamidi Jun 25 '14 at 16:01
Well, the code was obviously not well written. It obviously was not intended to function this way. It's old code, in an old windows forms app. I'm fixing it, but I was simply confused as to why it worked. – Erik Funkenbusch Jun 26 '14 at 04:14

i3arnon · Answer 3 · 2014-06-25T16:20:46.367

Well, apparently you can have null as the input to a using scope:

using (null)
{
    Console.WriteLine("hamster");
}

What happens is that _proxy is null and therefore _proxy as IDisposable is also null which makes the using scope redundant, no Dispose will be called (or attempted). The code is equivalent to this:

if (_proxy == null)
{
    ChannelFactory<IBasketService> factory =
            new ChannelFactory<IBasketService>("MyService");
    _proxy = factory.CreateChannel();
}
return _proxy;

It doesn't matter that _proxy is assigned to while inside the scope, Dispose would be "invoked" on the original copied value (which is null).

Here's a simple snippet that reproduces the issue. The "Disposed" message will not be written to console.

internal class Program
{
    private static DisposableScope _proxy = null;
    private static void Main(string[] args)
    {
        using (_proxy)
        {
            _proxy = new DisposableScope();
        }
    }
}

public class DisposableScope : IDisposable
{
    public void Dispose()
    {
        Console.WriteLine("Disposed");
    }
}

Why does this WCF Proxy code work?

3 Answers3