3

I've had troubles with System.Net.WebRequest and System.Net.HttpRequest in terms of compatibility with multi-threading and socket consumption. I've attempted to go a level lower and roll my own dirt simple Http classes.

Since the problem before was each thread was creating too many sockets too quickly, I am attempting to use a single socket (1 per thread) over multiple iterations ( a for loop).

Code:

My Test Class (has hardcoded ip and port until I can get it working):

public sealed class Foo : IDisposable {

        private string m_ip = "localhost";
        private int m_port = 52395;
        private TcpClient m_tcpClient;


        public Foo() {
            m_tcpClient = new TcpClient( m_ip, m_port );
        }

        public void Execute() {
            using( var stream = m_tcpClient.GetStream() )
            using( var writer = new StreamWriter( stream ) )
            using( var reader = new StreamReader( stream ) ) {
                writer.AutoFlush = true;
                // Send request headers
                writer.WriteLine( "GET /File HTTP/1.1" );
                writer.WriteLine( "Host: " + m_ip + ":" + m_port.ToString() );
                writer.WriteLine( "Connection: Keep-Alive" );
                writer.WriteLine();
                writer.WriteLine();

                // Read the response from server
                string response = reader.ReadToEnd();
                Console.WriteLine( response );
            }
        }    

        void IDisposable.Dispose() {
            m_tcpClient.Client.Dispose();
        }
    }

Static void Main:

using( Foo foo = new Foo() ) {
    for( int i = 0; i < 10; i++ ) {
        foo.Execute();
    }
}

Error

The error I am receiving is The operation is not allowed on non-connected sockets. after the first iteration of the for loop successfully completes.

I understand the cause of the error, (After the response is read the TcpClient.Client closes), but I don't know how to explicitly tell the socket to remain open.

Edit Further examination of the HTTP response I get back from the server it has Connection: Close in it. I assumed since this was raw TCP it wouldn't parse the HTTP. Could this be the root of the problem? (If so is there a way to ignore it)

James
  • 2,445
  • 2
  • 25
  • 35

3 Answers3

2

Change the order in your main method, so you will create a new object each iteration

for( int i = 0; i < 10; i++ ) 
{
    using( Foo foo = new Foo() ) 
    {
        foo.Execute();
    }
}

If you want to keep your socket open, you need to refactor your application a little bit so it will not call Dispose after one iteration, for example

public sealed class Foo : IDisposable {    
    private string m_ip = "localhost";
    private int m_port = 52395;
    private TcpClient m_tcpClient;

    private Stream stream;
    private StreamWriter writer;
    private StreamReader reader;

    public void Execute() {         
        // Send request headers
            ...    
        // Read the response from server                
    }   

    void Open(){
        m_tcpClient = new TcpClient( m_ip, m_port );
        stream = m_tcpClient.GetStream();
        writer = new StreamWriter( stream );
        reader = new StreamReader( stream );
    }   

    void Close() {
        m_tcpClient.Client.Dispose();
        reader.Dispose();
        writer.Dispose();
        stream.Dispose();
    }

    //Plus Dispose implementation
}

And here is the usage

using( Foo foo = new Foo() ) {
    foo.Open();
    for( int i = 0; i < 10; i++ ) {
        foo.Execute();
    }
    foo.Close();
}
oleksii
  • 35,458
  • 16
  • 93
  • 163
  • I read the question that he does _not_ want to create multiple instances. – Uwe Keim Jul 23 '12 at 17:40
  • While this will function in a synchronous program, it doesn't help me with socket consumption when I attempt to use it in a multi-threaded environment. (Socket clean-up by the OS doesn't match with thread iterations, thus a single thread will create more than 1 socket, eating up resources quickly) – James Jul 23 '12 at 17:41
  • `Dispose` should probably call `Close`, if you're gonna do it like this. I don't see a compelling reason not to just rename `Close` to `Dispose`, though. Symmetry with `Open`, maybe...but `Close` is actually just disposing, so... – cHao Jul 23 '12 at 17:49
  • @cHao yes I agree, but it is just a hint to OP, I tried to show that the connection should not be disposed of after each *for* step. There are also some blocking code missing, as well as exception handling and parameters validation etc. – oleksii Jul 23 '12 at 17:56
0

I think your implementation of KeepAlive isn't working quite right. When you are closing the stream, you are closing the underlying socket. Move your stream creation to the constructor.

public Foo() 
{
    m_tcpClient = new TcpClient( m_ip, m_port );
    m_tcpStream = m_tcpClient.GetStream();
}

Then keep both of them alive until the entire object is closed:

    void IDisposable.Dispose() 
    {
        m_tcpStream.Close();
        m_tcpClient.Client.Dispose();
    }
Ted Spence
  • 2,598
  • 1
  • 21
  • 21
  • This leads to the error `Stream was not writable` System.IO Exception, and the root of that is the TcpClient is still closing – James Jul 23 '12 at 17:57
  • I wonder if it's expecting a specific sequence of events in order to make the connection readable and writable? EDIT: Is there a reason you have to implement iDisposable? Why not try removing that and see if it behaves more predictably? – Ted Spence Jul 23 '12 at 19:25
0

When first Execute is invoked, using closes(disposes) m_tcpClient. You don't need those usings

using( var stream = m_tcpClient.GetStream() )
L.B
  • 114,136
  • 19
  • 178
  • 224