I only just noticed that doing something like this is wrong;

int thrdcount = 0;
while (thrdcount < 4)
{
    Thread t = new Thread(() =>
    {
        dowork(thrdcount); //Start the threadWorker
    });
    t.Start();
    thrdcount++; //Increase the thread count
}

Because the loop can end up finishing so fast that all 4 created threads get a 'thrdcount' with a value of 3. Or maybe two of them get a vlue of 2 and the other two a value of 3, etc.

So I changed it to this;

int thrdcount = 0;
while (thrdcount < 4)
{
    using (ManualResetEvent waitForThreadStart = new ManualResetEvent(false)) //Allows us to wait until the thread succesfully started
    {
        Thread t = new Thread(() =>
        {
            int threadcount_use = thrdcount;
            waitForThreadStart.Set(); //Let main thread know we started
            dowork(threadcount_use); //Start the threadWorker
        });
        t.Start();
        waitForThreadStart.WaitOne(); //Wait until the thread started
        thrdcount++; //Increase the thread count
    }
}

Work great! But now I'm wondering about the way I have a server accept new connections;

/// <summary>
/// Will accept connections until GlobalVars.mustClose is true or an exception occurs
/// </summary>
public void start()
{
    try
    {
        serverSocket = new TcpListener(port);
        serverSocket.Start();

        while (true)
        {
            TcpClient clientSocket = null; //Initiate TcpClient object that we will be sending to the client handler.

            if (GlobalVars.mustClose) break;
            try
            {
                bool cont = true;
                try
                {
                    output("Waiting for client to connect.");
                    clientSocket = serverSocket.AcceptTcpClient(); //Accept new client
                    if (clientConnect == null)
                    {
                        client_error("A client handler has not been set.");
                        continue;
                    }

                    //Get conn info
                    System.Net.Sockets.Socket s = clientSocket.Client;
                    string Cip = ((IPEndPoint)s.RemoteEndPoint).Address.ToString();
                    int Cport = ((IPEndPoint)s.RemoteEndPoint).Port;

                    //Output conn info
                    string client_con = String.Format("Client connected : {0}:{1}", Cip, Cport);
                    client_error(client_con);
                }
                catch (Exception) { cont = false; } //TODO: Proper error handling
                if (cont)
                {
                    if (!GlobalVars.mustClose)
                    {
                        Thread t = new Thread(() => 
                        {
                            clientConnect(clientSocket);
                        });
                        t.Start();
                    }
                }
            }
            catch (Exception) { client_error("Exception when connecting to client."); /*Failed to connect client*/ }
        }

        //This part of the code should never be reached...
        try
        {
            serverSocket.Stop();
        }
        catch (Exception) { } //Catch exception in case it already stopped earlier
    }
    catch (Exception) { fatal_error("Failed to start."); /*Failed to start*/ }
    fatal_error("Server ended.");
}

If the while loop accepts two clients really fast, is it possible for the 'clientConnect(clientSocket);' in the first loop to send the wrong (duplicate of second or null-valued) clientSocket, or will it at all times send the correct one, since a new one is created each loop?

Edited 4 Years Ago by crankyslap

You'll be fine. The first example is dealing with capture variables and threads. The second example doesn't deal with threads at all.

How do you mean 'It doesn't deal with threads'? It's starting a new thread after each client connect, passing the TcpClient object as an argument.

I've noticed that same problem before, too. But I don't think you rational behind why it occurs makes sense - how fast or how slow the threads fire shouldn't have an impact on the argument passed to the dowork() method.

I think it's a bug with the way annonymous delegates are optimized. To avoid that problem, I usually use the built in ParameterizedThreadStart class and pass it an object that contains the necessary information. In the first example, putting a Thread.Sleep(1) in the loop will likely correct the behaviour (it did in my test). This might be an explanation as to why your server works correctly - there's enough time in between each client connection to avoid the 'bug'.

That's just something I've noticed, it could be a complete lie with a really good explanation behind it. Regardless, it's kind of annoying.

This article has been dead for over six months. Start a new discussion instead.