116

I just want to know is it safe/ good approach to call return inside a using block.

For ex.

using(var scope = new TransactionScope())
{
  // my core logic
  return true; // if condition met else
  return false;
  scope.Complete();
}

We know the at the last most curly brace dispose() will get called off. But what will be in the above case, since return jumps the control out of the given scope (AFAIK)...

  1. Is my scope.Complete() get called?
  2. And so for the scope's dispose() method.
Paolo Moretti
  • 54,162
  • 23
  • 101
  • 92
  • 1
    Once the `using{}` scope is over, the relevant objects get disposed, `return` will "break" the scope - so the objects will get disposed as expected – Shai Aug 02 '12 at 12:02
  • 4
    Be aware that your `scope.Complete()` call will never be hit with the sample you provided, so you're transaction will always rollback. – Andy Aug 02 '12 at 16:08
  • Regardless of whether `using`'s `dispose()` is called, when the you return, the function containing this `using` block will have returned and everything belonging to it will be orphaned. So even if `scope` had not been disposed "by the `using`" (it will be, as others explained) it will be disposed anyway because the function ended. If C# had `goto` statement -are you done laughing yet? good- then instead of return you could `goto` to after the closing brace, without returning. Logically, `scope` would still be disposed, but you've just put `goto` in C# so who cares about logic at that stage. – Superbest Sep 01 '12 at 02:03
  • C# [has goto](https://msdn.microsoft.com/en-us/library/13940fs2(v=vs.71).aspx). – Jake T. Mar 13 '15 at 16:09
  • Related: [Returning in the middle of a using block](http://stackoverflow.com/q/662773/2157640) – Palec Mar 20 '15 at 14:52

6 Answers6

164

It's perfectly safe to call return inside your using block, since a using block is just a try/finally block.

In your example above after return true, the scope will get disposed and the value returned. return false, and scope.Complete() will not get called. Dispose however will be called regardless since it reside inside the finally block.

Your code is essentially the same as this (if that makes it easier to understand):

var scope = new TransactionScope())
try
{
  // my core logic
  return true; // if condition met else
  return false;
  scope.Complete();
}
finally
{
  if( scope != null) 
    ((IDisposable)scope).Dispose();
}

Please be aware that your transaction will never commit as there's no way to get to scope.Complete() to commit the transaction.

Øyvind Bråthen
  • 59,338
  • 27
  • 124
  • 151
  • 15
    You should make it clear that `Dispose` **will** get called. If the OP doesn’t know what happens in `using`, chances are he doesn’t know what happens with `finally`. – Konrad Rudolph Aug 02 '12 at 16:03
  • It is ok to leave a using block with return, but in case of TransactionScope you might will get in trouble with the using-statement itself: http://blogs.msdn.com/b/florinlazar/archive/2008/05/05/8459994.aspx – thewhiteambit Mar 26 '15 at 11:13
  • In my experience, this doesn't work with SQL Server CLR Assemblies. When I need to return results for a UDF containing a SqlXml field that is referencing a MemoryStream Object. I get "_Cannot access a disposed object_" and "_Invalid attempt to call Read when the stream is closed._", so I am FORCED to write leaky code and forgo the using-statement in this scenario. :( My only hope is the SQL CLR will handle the disposing of these objects. It's a unique scenario, but thought I'd share. – MikeTeeVee Dec 14 '16 at 08:48
  • 1
    @MikeTeeVee - Cleaner solutions are either (a) have the caller do the `using`, e.g. `using (var callersVar = MyFunc(..)) ..`, *instead* of having the using inside "MyFunc" - I mean the caller is given the stream and is responsible for closing it via `using` or explicitly, or (b) have MyFunc *extract* whatever info is needed into other objects, that can be safely passed back - then the underlying data objects or stream can be disposed by your `using`. You should not have to write leaky code. – ToolmakerSteve Jan 16 '17 at 18:31
7

That's fine - finally clauses (which is what the closing curly brace of the using clause does under the hood) always get executed when the scope is left, no matter how.

However, this is only true for statements that are in the finally block (which cannot be explicitly set when using using). Therefore, in your example, scope.Complete() would never get called (I expect the compiler to warn you about unreachable code though).

Lucero
  • 59,176
  • 9
  • 122
  • 152
2

In general, it is a good approach. But in your case, if you return before calling the scope.Complete(), it will just trash the TransactionScope. Depends to your design.

So, in this sample, Complete() is not called, and scope is disposed, assuming it is inheriting IDisposable interface.

M. Mennan Kara
  • 10,072
  • 2
  • 35
  • 39
2

scope.Complete should definitely be called before return. Compiler will display a warning and this code will never be called.

Regarding return itself - yes, it is safe to call it inside using statement. Using is translated to try-finally block behind the scene and finally block is to be certainly executed.

Tony Kh
  • 1,512
  • 11
  • 8
1

In the example you have provided, there is a problem; scope.Complete() is never called. Secondly, it is not a good practice to use return statement inside using statements. Refer to the following:

using(var scope = new TransactionScope())
{
    //have some logic here
    return scope;      
}

In this simple example, the point is that; the value of scope will be null when using statement is finished.

So it is better not to return inside using statements.

daryal
  • 14,643
  • 4
  • 38
  • 54
  • 1
    Just because 'return scope' is pointless, it doesn't imply that the return statement is wrong. – Preet Sangha Aug 02 '12 at 13:10
  • just because not using best practices does not imply you have done something wrong. It implies, it is best to avoid, as it may result in unforeseen consequences. – daryal Aug 02 '12 at 13:37
  • 1
    The value of `scope` will not be null - the only thing that will have happened is that `Dispose()` will have been invoked on that instance, and therefore the instance *should* not be used anymore (but it isn't null and there's nothing preventing you to try and use the disposed object, even though this indeed is an inappropriate use of a disposable object). – Lucero Aug 03 '12 at 16:50
  • Lucero is quite correct. Disposable objects are not null after being disposed. Its IsDisposed property is true, but if you check against null you get false and `return scope` returns a reference to _that_ object. This way, if you assign that reference on return you prevent the GC from cleaning up the disposed object. – ThunderGr Jan 05 '13 at 09:34
0

In this example, scope.Complete() will never execute. However, the return command will cleanup everything that is assigned on the stack. The GC will take care of everything that is unreferenced. So, unless there is an object that can not be picked up by the GC, there is no problem.

ThunderGr
  • 2,297
  • 29
  • 20