views:

278

answers:

3

I'm working on a web server in C# and I have it running on Asynchronous socket calls. The weird thing is that for some reason, when you start loading pages, the 3rd request is where the browser won't connect. It just keeps saying "Connecting..." and doesn't ever stop. If I hit stop. and then refresh, it will load again, but if I try another time after that it does the thing where it doesn't load again. And it continues in that cycle. I'm not really sure what is making it do that.

The code is kind of hacked together from a couple of examples and some old code I had. Any miscellaneous tips would be helpful as well.

Heres my little Listener class that handles everything

(pastied here. thought it might be easier to read this way)

using System;
using System.Collections.Generic;
using System.Net;
using System.Net.Sockets;
using System.Text;
using System.Threading;
using irek.Request;
using irek.Configuration;
namespace irek.Server
{
    public class Listener
    {
        private int port;
        private Socket server;
        private Byte[] data = new Byte[2048];
        static ManualResetEvent allDone = new ManualResetEvent(false);
        public Config config;

        public Listener(Config cfg)
        {
            port = int.Parse(cfg.Get("port"));
            config = cfg;
            ServicePointManager.DefaultConnectionLimit = 20;
        }

        public void Run()
        {
            server = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
            IPEndPoint iep = new IPEndPoint(IPAddress.Any, port);
            server.Bind(iep);

            Console.WriteLine("Server Initialized.");
            server.Listen(5);
            Console.WriteLine("Listening...");
            while (true)
            {
                allDone.Reset();
                server.BeginAccept(new AsyncCallback(AcceptCon), server);
                allDone.WaitOne();
            }

        }

        private void AcceptCon(IAsyncResult iar)
        {
            allDone.Set();
            Socket s = (Socket)iar.AsyncState;
            Socket s2 = s.EndAccept(iar);
            SocketStateObject state = new SocketStateObject();
            state.workSocket = s2;
            s2.BeginReceive(state.buffer, 0, SocketStateObject.BUFFER_SIZE, 0, new AsyncCallback(Read), state);
        }

        private void Read(IAsyncResult iar)
        {
            try
            {
                SocketStateObject state = (SocketStateObject)iar.AsyncState;
                Socket s = state.workSocket;

                int read = s.EndReceive(iar);

                if (read > 0)
                {
                    state.sb.Append(Encoding.ASCII.GetString(state.buffer, 0, read));

                    SocketStateObject nextState = new SocketStateObject();
                    nextState.workSocket = s;
                    s.BeginReceive(state.buffer, 0, SocketStateObject.BUFFER_SIZE, 0, new AsyncCallback(Read), nextState);
                }
                if (state.sb.Length > 1)
                {
                    string requestString = state.sb.ToString();
                    // HANDLE REQUEST HERE
                    byte[] answer = RequestHandler.Handle(requestString, ref config);
                    // Temporary response
                    /*
                    string resp = "<h1>It Works!</h1>";
                    string head = "HTTP/1.1 200 OK\r\nContent-Type: text/html;\r\nServer: irek\r\nContent-Length:"+resp.Length+"\r\n\r\n";
                    byte[] answer = Encoding.ASCII.GetBytes(head+resp);
                    // end temp.
                    */
                    state.workSocket.BeginSend(answer, 0, answer.Length, SocketFlags.None, new AsyncCallback(Send), s);
                }
            }
            catch (Exception e)
            {
                Console.WriteLine(e.Message);
                Console.WriteLine(e.StackTrace);
                return;
            }
        }

        private void Send(IAsyncResult iar)
        {
            try
            {
                SocketStateObject state = (SocketStateObject)iar.AsyncState;
                int sent = state.workSocket.EndSend(iar);
                state.workSocket.Shutdown(SocketShutdown.Both);
                state.workSocket.Close();
            }
            catch (Exception)
            {

            }
            return;
        }
    }
}

And my SocketStateObject:

public class SocketStateObject
{
    public Socket workSocket = null;
    public const int BUFFER_SIZE = 1024;
    public byte[] buffer = new byte[BUFFER_SIZE];
    public StringBuilder sb = new StringBuilder();
}

** EDIT **

I have updated the code with some suggestions from Chris Taylor.

+1  A: 

Completely random guess:

http://msdn.microsoft.com/en-us/library/system.net.servicepointmanager.defaultconnectionlimit.aspx

The maximum number of concurrent connections allowed by a ServicePoint object. The default value is 2.

Brian
This was a good thought, but not it. The connections I'm trying aren't concurrent anyways. They are separate, one after another. Though I did think that for some reason, the connections weren't getting closed, so it made sense. But alas, not the case :/
The.Anti.9
+1  A: 

Just looking at the code quickly, I suspect that you might stop enquing your AsyncReads because s.Available is returning 0, I am refering to the following code

if (read > 0)
{
    state.sb.Append(Encoding.ASCII.GetString(state.buffer, 0, read));
    if (s.Available > 0) 
    { 
        s.BeginReceive(state.buffer, 0, SocketStateObject.BUFFER_SIZE, 0, new   AsyncCallback(Read), state); 
        return; 
    } 
}

To confirm, change the above to the following

if (read > 0)
{
  state.sb.Append(Encoding.ASCII.GetString(state.buffer, 0, read));

  SocketStateObject nextState = new SocketStateObject();
  nextState.workSocket = s;
  s.BeginReceive(state.buffer, 0, SocketStateObject.BUFFER_SIZE, 0, new AsyncCallback(Read), nextState);
}

This is not the complete correction of the code, but it will confirm if this is the problem. You need to make sure that you are closing your sockets correctly etc.

Update I also noticed that you are sending the socket in as the state in the call to BeginSend.

 state.workSocket.BeginSend(answer, 0, answer.Length, SocketFlags.None, new AsyncCallback(Send), state.workSocket); 

However, your callback Send is casting the AsyncState to SocketStateObject

SocketStateObject state = (SocketStateObject)iar.AsyncState; 

This will be raising InvalidCastExceptions which you are just hiding by adding the empty catch. I am sure others will agree, this is exceptionally bad practice having empty catches it hides so much info that you could be using to debug your problem.

Chris Taylor
No, unfortunately, it didn't change. So I don't think this was my problem either. But this may have prevented other bugs :P
The.Anti.9
@The.Anti.9, that is strange. I just copied your original code exactly and could replicate your problem. I then added the change I suggested and from that point the problem when away. Maybe you can post the updated code?
Chris Taylor
@Chris Taylor ok I updated it. I just copied and pasted the code you put over top of mine, so I don't think i made any mistakes. though I could have. I have been working on other parts of the app, so now i think we need the `return` cause if not it goes on and tries to parse an empty request. and that throws an exception.
The.Anti.9
ah HA! that was it, it was that exception. I need to be more careful with my error handling.
The.Anti.9
A: 

You should also note that there is a race condition in your code. In Run(), you wait for allDone before calling BeginAccept again:

while (true)
{
     allDone.Reset();
     server.BeginAccept(new AsyncCallback(AcceptCon), server);
     allDone.WaitOne(); // <------
}

This is fine, however in your AcceptConn callback, the event is set at the top of the method:

private void AcceptCon(IAsyncResult iar)
{
    allDone.Set(); // <------

    Socket s = (Socket)iar.AsyncState;
    Socket s2 = s.EndAccept(iar);
    SocketStateObject state = new SocketStateObject();
    state.workSocket = s2;
    s2.BeginReceive(state.buffer, 0, SocketStateObject.BUFFER_SIZE, 0,
        new AsyncCallback(Read), state);
}

The callback will executed by a random thread from the pool, but allDone will be set before anything is actually done. It's entirely possible for your Run() loop to run again in the first thread before the work in AcceptCon actually completes. This will cause you big problems.

You should set allDone after you've performed your initialization (and especially after you've accessed any non-threadsafe class members), like so:

private void AcceptCon(IAsyncResult iar)
{

    Socket s = (Socket)iar.AsyncState;
    Socket s2 = s.EndAccept(iar);
    SocketStateObject state = new SocketStateObject();
    state.workSocket = s2;

    allDone.Set(); // <------

    s2.BeginReceive(state.buffer, 0, SocketStateObject.BUFFER_SIZE, 0,
        new AsyncCallback(Read), state);

}
jscharf