CODEWITHSUNDEEP
c#return-value
Sangeetha
Sangeetha

Reputation: 709

return statement before finally

I have a c# program which acts as client and many client program which is again a c# windows application connects to this c# server program to read data from sqlite database. To avoid lock issues when connecting multiple clients i have used below code,

System.Threading.Monitor.Enter(Lock);

try                       
{                     
    filter.Execute();//get data from database
    Request.Clear();
    return filter.XML;//create xml and return to client
}
finally
{
    System.Threading.Monitor.Exit(Lock);
}

server gets hangs some times and need to restart the server program. Is it a good practce to make the return statement before finally?

Regards sangeetha

Upvotes: 5

Views: 1485

Answers (4)

Andy Brown
Andy Brown

Reputation: 19161

As you have no catch block, there is no guarantee that finally will be executed. From MSDN - try-finally (C# Reference) and "Locks and exceptions do not mix" (Eric Lippert)

Within a handled exception, the associated finally block is guaranteed to be run. However, if the exception is unhandled, execution of the finally block is dependent on how the exception unwind operation is triggered. That, in turn, is dependent on how your computer is set up.

And from the link that is then mentioned (Unhandled Exception Processing In The CLR) there are then various considerations which could mean you end up with a terminated thread. I don't honestly know if that would leave you with a lock on the lock object though.

If you wanted to ensure that:

  • you release the lock; but
  • you don't want to handle the exception at this level, instead you want it handled by a higher level exception handler

Then do:

TheXmlType xml = null;
Monitor.Enter(Lock);
bool inLock = true;
try {
  ...
  xml = filter.Xml; // put this here in case it throws an exception
  inLock = false; // set this here in case monitor.exit is to 
    // throw an exception so we don't do the same all over again in the catch block
  Monitor.Exit(Lock);
  return xml; // this is fine here, but i would normally put it outside my try
}
catch (Exception) {
  if (inLock) Monitor.Exit(Lock);
  throw;
}

But, please note: don't use catch (Exception) to hide exceptions, this is only ok if you are re-throwing the exception. People also recommend you have a single return statement, and usually this would be outside your try block.

EDIT:

Confirmed with a test program, and from MSDN - Exceptions in Managed Threads

Starting with the .NET Framework version 2.0, the common language runtime allows most unhandled exceptions in threads to proceed naturally. In most cases this means that the unhandled exception causes the application to terminate.

So, if you don't handle your exception, your application will crash (and you won't have to worry about the lock). If you do handle it, then your original code would execute it's finally block and you are ok.

EDIT 2: Test code updated as it did not properly illustrate a non-firing finally:

class Program
{ 
    static void Main(string[] args) {
        Program p =new Program();
        p.Start();
        Console.WriteLine("done, press enter to finish");
        Console.ReadLine();
    }

    private readonly object SyncRoot = new object();
    ManualResetEvent mre = new ManualResetEvent(false);

    private void Start() {
        /*
         * The application will run the thread, which throws an exception
         * While Windows kicks in to deal with it and terminate the app, we still get 
         * a couple of "Failed to lock" messages
         * */

        Thread t1 = new Thread(SetLockAndTerminate);
        t1.Start();
        mre.WaitOne();
        for (int i = 0; i < 10; i++) {
            if (!Monitor.TryEnter(this.SyncRoot, 1000)) {
                Console.WriteLine("Failed to lock");
            }
            else {
                Console.WriteLine("lock succeeded");
                return;
            }
        }
        Console.WriteLine("FINALLY NOT CALLED");
    }
    public int CauseAnOverflow(int i)
    {
        return CauseAnOverflow(i + 1);
    }
    public void SetLockAndTerminate() {
        Monitor.Enter(this.SyncRoot);
        Console.WriteLine("Entered");
        try {
            mre.Set();
            CauseAnOverflow(1); // Cause a stack overflow, prevents finally firing
        }
        finally {
            Console.WriteLine("Exiting");
            Monitor.Exit(this.SyncRoot);
        }
    }
}

Upvotes: 1

Mojtaba
Mojtaba

Reputation: 1240

Is it bad practice to return from within a try catch finally block?
It is the right way of writing Exception Handling in c# and always finally block will be executed it does not depend on the place of return. I do not know about your code but you should find your problem somewhere else (for example if your code was hosted in IIS I would suspect the state of Lock object in different loaded domains or maybe the lock is just for one incoming call and it happens in database or what is Request.Clear() you don't have a lock block there?). You can easily log the state of incoming calls and find the problem.

Upvotes: 0

VladL
VladL

Reputation: 13033

Yes, it's what finally statement is for. It will be executed after the return, even if an exception occurs

EDIT: this simple code will show you, that catch block is not needed for finally block to be executed

public Form1()
{
    InitializeComponent();
    check();
}

private string check()
{
    try
    {
        return String.Empty;
    }
    finally
    {
        MessageBox.Show("finally");
    }
}

Upvotes: 2

bash.d
bash.d

Reputation: 13207

From MSDN

By using a finally block, you can clean up any resources that are allocated in a try block, and you can run code even if an exception occurs in the try block. Typically, the statements of a finally block run when control leaves a try statement. The transfer of control can occur as a result of normal execution, of execution of a break, continue, goto, or return statement, or of propagation of an exception out of the try statement.

Within a handled exception, the associated finally block is guaranteed to be run. However, if the exception is unhandled, execution of the finally block is dependent on how the exception unwind operation is triggered. That, in turn, is dependent on how your computer is set up. For more information, see Unhandled Exception Processing in the CLR.

Upvotes: 4

Related Questions