views:

219

answers:

5

I'm writing a program which listens to an incoming TcpClient and handles data when it arrives. The Listen() method is run on a separate thread within the component, so it needs to be threadsafe. If I break out of a do while loop while I'm within a lock() statement, will the lock be released? If not, how do I accomplish this?

Thanks!

(Any other advice on the subject of Asynchronous TCP Sockets is welcome as well.)

private void Listen()
{
    do
    {
        lock (_clientLock)
        {
            if (!_client.Connected) break;
            lock (_stateLock)
            {
                if (!_listening) break;
                if (_client.GetStream().DataAvailable) HandleData();
            }
        }
        Thread.Sleep(0);
    } while (true);
}
+1  A: 

Once you exit the lock{}, it will unlock what you have locked (it's just like a using statement in that regard). It doesn't matter where you exit (the beginning, the end, or the middle), it's that you left the scope of the lock at all. Think about what would happen if you raised an exception in the middle.

Kevin
Very important to think about what would happen if you raised an exception in the middle. Something exceptional and unexpected goes wrong in the middle of a critical operation and what's the first thing you do? *Release the lock* so that other code can go in and access the resource that caused an exception in the middle of a critical operation! The fact that locks are released on exceptions means that you have *more* work to do, not *less*.
Eric Lippert
That is true, I wasn't thinking of it that way. That is an excellent point. My thought was what if it didn't release the lock. With an exception, it's unpredictable where in the stack it will get caught. If someone wasn't careful, the exception could bubble several levels where there is no hint that a lock was ever used. Now there is an exception to deal with and a locked object which is preventing "potentially" other successful operations from occurring.
Kevin
The trouble is that something awful can happen no matter what strategy you choose. Locks and exceptions mix poorly. If we had cheap Software Transactional Memory it would be less of a problem; we could simply roll back memory to the state it was before the lock was entered. But we don't have that tool in our toolbox yet.
Eric Lippert
+10  A: 

Yes. The lock statement translates into a try/finally clause. In C# 4, for example, a lock statement like so:

lock(obj)
{
    // body
}

roughly translates (taken from Eric Lippert's blog here) to:

bool lockWasTaken = false;
var temp = obj;
try 
{ 
    Monitor.Enter(temp, ref lockWasTaken); 
    { 
       // body 
    }
}
finally 
{ 
    if (lockWasTaken) 
        Monitor.Exit(temp); 
}

When the execution leaves the scope of the lock {}, the underlying lock will be released automatically. This will happen no matter how you exit scope (break/return/etc), since the call to Monitor.Exit is wrapped, internally, inside of the finally block of a try/finally.

Reed Copsey
Faster than me you are :)
Brian Rasmussen
+3  A: 

Yes, the lock will be released. You can use ILDASM or Reflector to look at the actual generated code. The lock statement is shorthand for the following code (roughly).

Monitor.Enter(_client);
try
{
  // do your stuff

}
finally {
  Monitor.Exit(_client);
}

Notice the finally block is always executed.

Haacked
BTW - this is correct for <= C#3, but not quite what happens in C# 4.
Reed Copsey
Yep, Eric Lippert documents the change in C# 4. http://blogs.msdn.com/ericlippert/archive/2009/03/06/locks-and-exceptions-do-not-mix.aspx
Haacked
Note that the finally block is *not* always executed. The finally block is only executed *if control leaves the try*. Control might not leave the try; the try block might contain an infinite loop. Another thread might FailFast the process. An administrator might kill the process. A thread might hit the stack guard page twice in a row, taking the process down. Someone might unplug the machine. In all of these situations the finally block does not execute.
Eric Lippert
@Eric Lippert: You left out cosmic rays altering voltages in the CPU :)
LBushkin
Hmmm, seeing as how Eric Lippert is on StackOverflow, I must be much more precise in my descriptions. :)
Haacked
+1  A: 

Because you asked for other advice...I noticed that you are nesting locks. This, by itself, is not necessarily a bad thing. But, it is one my red flags I watch out for. There is the possibility of a deadlock if you ever acquire those two locks in a different order in another part of your code. I am not saying there is anything wrong with your code. It is just something else to watch out for because it is easy to get wrong.

Brian Gideon
Thanks for the warning! I'm making sure to lock objects in the same order, tho, so that it won't deadlock.
Daniel Rasmussen
A: 

To answer the other half of your question:

Any other advice on the subject of Asynchronous TCP Sockets is welcome as well

Simply put I wouldn't manage this in the fashion demonstrated by your original post. Rather seek help from the System.Net.Sockets.TcpClient and the System.Net.Sockets.TcpListener classes. Use the async calls like BeginAcceptSocket(...) and BeginRead(...) and allow the ThreadPool to do it's job. It's really pretty easy to put together that way.

You should be able to achieve all the server behavior you desire without ever coding the dreaded words "new Thread" :)

Here is a basic example of the idea, minus the idea of graceful shutdown, exception handling ect:

public static void Main()
{
    TcpListener listener = new TcpListener(new IPEndPoint(IPAddress.Loopback, 8080));
    listener.Start();
    listener.BeginAcceptTcpClient(OnConnect, listener);

    Console.WriteLine("Press any key to quit...");
    Console.ReadKey();
}

static void OnConnect(IAsyncResult ar)
{
    TcpListener listener = (TcpListener)ar.AsyncState;
    new TcpReader(listener.EndAcceptTcpClient(ar));
    listener.BeginAcceptTcpClient(OnConnect, listener);
}

class TcpReader
{
    string respose = "HTTP 1.1 200\r\nContent-Length:12\r\n\r\nHello World!";
    TcpClient client;
    NetworkStream socket;
    byte[] buffer;

    public TcpReader(TcpClient client)
    {
        this.client = client;
        socket = client.GetStream();

        buffer = new byte[1024];
        socket.BeginRead(buffer, 0, 1024, OnRead, socket);
    }

    void OnRead(IAsyncResult ar)
    {
        int nBytes = socket.EndRead(ar);
        if (nBytes > 0)
        {
            //you have data... do something with it, http example
            socket.BeginWrite(
                Encoding.ASCII.GetBytes(respose), 0, respose.Length, null, null);

            socket.BeginRead(buffer, 0, 1024, OnRead, socket);
        }
        else
            socket.Close();
    }
}

For a much more complicated example of how to do this see the SslTunnel Library I wrote a while ago.

csharptest.net