0

Yesterday I came accross a strange problem, which gave me quite some headaches. I have a server application with a Server class, which in turn is derived from a Connection class. The Connection class provides information about the connection state and the possibility to close the connection

public bool Connected
{
    get
    {
        if (connection != null)
        {
            lock (lockObject)
            {
                bool blockingState = connection.Blocking;

                try
                {
                    connection.Blocking = false;
                    connection.Send(new byte[1], 1, 0);
                }
                catch (SocketException e)
                {
                    if (!e.NativeErrorCode.Equals(10035))
                    {
                        return false;
                    }
                     //is connected, but would block

                }
                finally
                {
                    connection.Blocking = blockingState;
                }

                return connection.Connected;

            }

        }

        return false;
   }
}

public virtual void CloseConnection()
{
    if (Connected)
    {
        lock (lockObject)
        {
            connection.Close();
        }
    }
}

The Server class is resonsible for actually sending data

private void ConnectAndPollForData()
{
    try
    {
        TcpListener listener = new TcpListener(Port);

        listener.Start();
        while (true)
        {
            connection = listener.AcceptSocket();

            string currentBuffr = string.Empty;
            const int READ_BUFFER_SIZE = 1024;
            byte[] readBuffr = new byte[READ_BUFFER_SIZE];

            while (Connected)
            {
                int bytesReceived;

                lock (lockObject)
                {
                    bytesReceived = connection.Receive(readBuffr, READ_BUFFER_SIZE, SocketFlags.None);
                }
                currentBuffr += ASCIIEncoding.ASCII.GetString(readBuffr, 0, bytesReceived);

                //do stuff

        }
    }
    catch(ThreadAbortException)
    {
        Thread.ResetAbort();
    }
    finally
    {

    }
}

public void SendString(string stringToSend)
{
    stringToSend += "\r\n";


    if(Connected)
    {
        lock(lockObject)
        {
            connection.Send(ASCIIEncoding.UTF7.GetBytes(stringToSend));
        }
    }

}

There is no other explicit access to the connection object. The ConnectAndPollForData function executes in a seperate thread. Whenever I ran the host in this version (I am currently using a non thread-safe version, which causes other problems) it hang after quite a few lines received via TCP. Pausing the debugger showed me, that one thread tried to execute the code withing the lock of Connected, while the other tried to receive data in the lock of ConnectAndPollForData. This behavior seems strange to me, for I would expect to execute the code within the first lock and then do the second. There do seem to be similar problems when using callbacks like in Deadlocking lock() method or 'Deadlock' with only one locked object? but the situation here is a bit different, for in my situation (I think) the code within the locks should not emit any events that themselves try to obtain a lock on the object.

Community
  • 1
  • 1
Paul Kertscher
  • 9,416
  • 5
  • 32
  • 57

1 Answers1

4

Let's assume it gets the lock in the second method first. So it is holding the lock, and waiting for data. It is unclear whether this is directly receiving the data sent by the first method, or whether this is looking for a reply from an unrelated server - a reply to the message sent in the first method. But either way, I'm assuming that there won't be data incoming until the outbound message is sent.

Now consider: the outbound message can't be sent, because you are holding an exclusive lock.

So yes, you've deadlocked yourself. Basically, don't do that. There is no need to synchronize between inbound and outbound socket operations, even on the same socket. And since it makes very little sense to have concurrent readers on the same socket, or concurrent writers, I'm guessing you don't actually need those locks at all.

Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900