send problem in threads

Started by
4 comments, last by Drew_Benton 13 years, 6 months ago
I have been trying to make a 'bot' in vis c# 2008

I have noticed that after I connect, If I send the appropriate packets by pressing buttons on the form window, (so send command is called from main form) then everything works fine

However, If I have my bot respond to packets recieved, and sends the correct packets from the threads that read the recieved data, then for some reason, my connection gets dc after about 2 min, and then if i try to connect with any other app, its acts like my port or whatever, is blocked.

And before anyone asks, yes, the packets are identical in both versions, because I am calling the same functions... just in the first example, from a putton click, second example, from inside a thread.

So has anyone has this issue before? How should I fix this? Thanks
Advertisement
Perhaps your asynchronous messages get jumbled in the buffer. This is especially likely if you send bits of commands at a time.

As a test, try wrapping the sending function in a lambda, and call Invoke on the main form to have them called in the main thread. If this causes deadlocks, try using BeginInvoke() instead, although that will generate dangling references unless you can call EndInvoke() "sufficiently later" without causing deadlocks.

Another test would be to surround all the sending code with lock() on a common object (such as the sending socket).
enum Bool { True, False, FileNotFound };
I will try the lock method and see if I get any results.

However... would it be 'good' programming practice to have a timer on my main form, set for 10 ms, per tick

then have the whole connection defined in class, and created as an object (because I would like to have multiple client connections)

so every tick, just have the main application loop through the client object, and have each one ... query the socket for data, and then process and act upon that data?

Is there any type of tutorial showing how you could set up a single app, having multiple client objects, each one being able to connect, disconnect, read and send data on their own?

Thanks
In C#, you want to use async I/O on sockets, such as BeginReceive() or similar.
You also want to send data from only one thread at a time, per socket, using either locking or a worker thread.
enum Bool { True, False, FileNotFound };
This is very frustrating, the locking didnt help, and even using async sockets didnt help.

let me try to explain what its doing in pseudo code

ok, i have a form
i have a class...

cPlayer
{
Socket client;
int userstate=0;
}

the class has 4 functions
connect()
read();
send();
process();

if I do this....
create a cPlayer object, call the connect() function
the connect funciton starts 2 threads, read and process
read reads in data from the socket and places it in a queueu
process reads the data from the queue and processes it

the above way fails, the player will log into th server, but dc after 2 min, and any packets i try to send will cause a dc

-----------------------
if I do the exact same thing as above (but instead starting a process thread) I have buttons on the main from that .... request login, send username and pw, select game character

This way it will work perfectly

-----------------------
if I do the same as above, but instead of the process thread, or the buttons, I have a timer, that calls the functions for (login reques, username & pw, slect char) and calls them one after another , each 1 second apart

It works this way as well

------------------------
Now the last way I tried it, is I start the 'process' thread, but instead of calling the functions, I have the process thread, set the UserState variable in the cPlayer object

Then the timer will look at the userState and depending on what it is, call the correct function

This way, the character logs into the game, but we have the dc after 2 min, and cant send any other packets, or dc

-------------------------

The functions I call are identical, so it only seems to be where I am calling them from. I have the socket variable in the cPlayer class, and the connection function, also the login reques, send un & pw, select game char functions are also in the cPlayer class. However, still it seems that I have to use the pure timer, or buttons on the main form to get it to work.
There are a lot of subtleties when using C# asynchronous sockets.

The key things to remember are:
- Each client context needs its own recv/send buffers.
- If you store a global client list for regular enumeration, you have to lock it.
- Sending data is more complicated in this scheme and requires locking.
- Receiving does not require inherent locking as long as you setup the order of operations correctly. However if you queue packets for a processing thread like you mention, a lock is required on that structure.
- Error handling can become a nightmare trying to track and catch exceptions and keep the connection in a deterministic state!

Here is a small example I've whipped up that should show most of the concepts you need to be aware of when using C# for the direction you are going. I've not really done extensive C# programming so there may be a few bugs here and there or there might be easier ways to do such things, but the concepts involving TCP use should be solid.

using System;using System.Collections.Generic;using System.Linq;using System.Text;using System.Net;using System.Net.Sockets;using System.IO;using System.Runtime.InteropServices;using System.Threading;// TODO: Proper socket cleanup for clients that disconnect!namespace ConsoleApplication2{    class Program    {        [DllImport("crtdll.dll")]        public static extern int _kbhit();        class Player        {            public int id = -1;            public Socket s = null;            public byte[] data = new byte[8192];            public List<byte[]> pending_read_packets = new List<byte[]>();            public int pending_read_size = 0;            public List<byte[]> pending_write_packets = new List<byte[]>();            public int pending_write_size = 0;            public object write_in_progress_lock = new object();            public bool write_in_progress = false;            public void Send(byte[] outgoing)            {                lock (write_in_progress_lock) // Have to sync write stuff                {                    pending_write_packets.Add(outgoing); // Add the packet                    if (write_in_progress == false) // If no write going on, start a new write, otherwise handler does it for us                    {                        write_in_progress = true;                        s.BeginSend(pending_write_packets[0], 0, pending_write_packets[0].Length, SocketFlags.None, new AsyncCallback(SendData), this);                    }                }            }        }        static Dictionary<int, Player> players;        static void SendData(IAsyncResult iar)        {            Player player = (Player)iar.AsyncState;            try            {                int sent = player.s.EndSend(iar);                lock (player.write_in_progress_lock) // Have to sync write stuff                {                    player.pending_write_size += sent;                    if (player.pending_write_packets[0].Length != player.pending_write_size) // Partial write, continue sending data                    {                        player.s.BeginSend(player.pending_write_packets[0], player.pending_write_size, player.pending_write_packets[0].Length - player.pending_write_size, SocketFlags.None, new AsyncCallback(SendData), player);                        return;                    }                    player.pending_write_packets.RemoveAt(0); // All done with this packet                    player.pending_write_size = 0;                    if (player.pending_write_packets.Count != 0) // Start sending the next packet                    {                        player.s.BeginSend(player.pending_write_packets[0], 0, player.pending_write_packets[0].Length, SocketFlags.None, new AsyncCallback(SendData), player);                    }                    else                    {                        player.write_in_progress = false; // No more data to write                    }                }            }            catch (System.Exception ex)            {                System.Console.WriteLine(ex); // TODO: Invoke an "OnError" delegate with the 'player' object.                lock (players)                {                    try                    {                        players.Remove(player.id);                    }                    catch (System.Exception ex2)                    {                        System.Console.WriteLine(ex2); // TODO: Error handling                    }                }            }        }        static void ReceiveData(IAsyncResult iar)        {            const int header_size = 2; // 2 is just the size I choose for the fixed size packet header            Player player = (Player)iar.AsyncState;            try            {                int recv = player.s.EndReceive(iar);                if (recv == 0) // Client disconnected                {                    // TODO: Invoke an "OnDisconnect" delegate with the 'player' object.                    Console.WriteLine("Client {0} has disconnected.", player.id);                    lock (players)                    {                        try                        {                            players.Remove(player.id);                        }                        catch (System.Exception ex2)                        {                            System.Console.WriteLine(ex2); // TODO: Error handling                        }                    }                    return;                }                player.pending_read_size += recv;                if (player.pending_read_size < header_size)                {                    player.s.BeginReceive(player.data, player.pending_read_size, player.data.Length - player.pending_read_size, SocketFlags.None, new AsyncCallback(ReceiveData), player);                    return;                }                MemoryStream memory = new MemoryStream(player.data);                BinaryReader reader = new BinaryReader(memory);                int data_size = reader.ReadUInt16(); // Could be a simpler way, but this is straight forward and clearly shows what is going on                if (player.pending_read_size < data_size + header_size) // Need to make sure we have the entire header + data                {                    player.s.BeginReceive(player.data, player.pending_read_size, player.data.Length - player.pending_read_size, SocketFlags.None, new AsyncCallback(ReceiveData), player);                    return;                }                byte[] packet = new byte[data_size + header_size];                Buffer.BlockCopy(player.data, 0, packet, 0, data_size + header_size);                lock (player.pending_read_packets) // queue the packet for the processing thread                {                    // NOTE: This is not actually needed since async will spawn more worker threads for you                    // if more networking events happen and all current worker threads are blocked doing client                    // processing stuff. However, if you limit your client worker thread pool count so it won't                    // "explode" in that case, then this concept *is* needed.                    // Just debugging                    Console.WriteLine("Client {0} received:");                    for (int x = 0; x < packet.Length; ++x)                    {                        Console.Write("{0:X2} ", packet[x]);                        if ((x + 1) % 16 == 0)                        {                            Console.WriteLine();                        }                    }                    Console.WriteLine();                    player.pending_read_packets.Add(packet); // Or process the packet as needed                }                player.pending_read_size -= (data_size + header_size);                if (player.pending_read_size > 0) // Annoying part of TCP stream processing!                {                    Buffer.BlockCopy(player.data, data_size + header_size, player.data, 0, player.pending_read_size);                }                player.s.BeginReceive(player.data, player.pending_read_size, player.data.Length - player.pending_read_size, SocketFlags.None, new AsyncCallback(ReceiveData), player);            }            catch (System.Exception ex)            {                System.Console.WriteLine(ex); // TODO: Invoke an "OnError" delegate with the 'player' object.                lock (players)                {                    try                    {                        players.Remove(player.id);                    }                    catch (System.Exception ex2)                    {                        System.Console.WriteLine(ex2); // TODO: Error handling                    }                }            }        }        static void Connected(IAsyncResult iar)        {            Player player = (Player)iar.AsyncState;            try            {                player.s.EndConnect(iar);                // TODO: Invoke an "OnConnect" delegate with the 'player' object signaling success.                // NOTE: If you do anything with global state you need to properly lock it first!                Console.WriteLine("Client {0} has connected.", player.id);                player.s.BeginReceive(player.data, player.pending_read_size, player.data.Length - player.pending_read_size, SocketFlags.None, new AsyncCallback(ReceiveData), player);            }            catch (SocketException ex)            {                // TODO: Invoke an "OnConnect" delegate with the 'player' object signaling an error.                System.Console.WriteLine(ex);                lock (players)                {                    try                    {                        players.Remove(player.id);                    }                    catch (System.Exception ex2)                    {                        System.Console.WriteLine(ex2); // TODO: Error handling                    }                }            }        }        static void Main(string[] args)        {            players = new Dictionary<int, Player>();            IPEndPoint iep = new IPEndPoint(IPAddress.Parse("127.0.0.1"), 1234);            for (int x = 1; x <= 1; ++x)            {                try                {                    Player player = new Player();                    player.id = x;                    player.s = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);                    player.s.BeginConnect(iep, new AsyncCallback(Connected), player);                    lock (players)                    {                        players.Add(player.id, player);                    }                }                catch (System.Exception ex)                {                    System.Console.WriteLine(ex); // TODO: Error handling                }            }            while (_kbhit() == 0) // Console busy loop, won't have with a GUI, so I just use this as your "process thread" instead.            {                lock (players)                {                    foreach (KeyValuePair<int, Player> player in players)                    {                        lock (player.Value.pending_read_packets)                        {                            foreach (byte[] packet in player.Value.pending_read_packets)                            {                                MemoryStream memory = new MemoryStream(packet);                                BinaryReader reader = new BinaryReader(memory);                                // TODO: process the packet                                byte[] outgoing1 = new byte[1]; // Let's say we wanted to send these packets in response                                byte[] outgoing2 = new byte[1];                                player.Value.Send(outgoing1);                                player.Value.Send(outgoing2);                            }                            player.Value.pending_read_packets.Clear();                        }                    }                }                Thread.Sleep(100); // process packets every 100ms                                // TODO: You'd want to implement a more efficient system based on your needs                // later. Either Sleep 1 to prevent 100% cpu use for the thread or have some                // sort of blocking signal to trigger the checking logic as simply locking                // all this stuff can lead to issues if not done correctly.            }            lock (players)            {                foreach (KeyValuePair<int, Player> player in players)                {                    try                    {                        player.Value.s.Shutdown(SocketShutdown.Both);                    }                    catch (System.Exception ex)                    {                        System.Console.WriteLine(ex); // TODO: Error handling                    }                }            }            // Wait up to 30s for all clients to exit            for (int x = 0; x < 30; ++x )            {                lock (players)                {                    if (players.Count == 0)                    {                        break;                    }                }                Console.WriteLine("[Attempt {0}] Waiting for all clients to finish.", x + 1);                Thread.Sleep(1000);            }        }    }}


The hardest part is keeping track of what you have to lock and what you do not. Then you have to worry about making sure you don't come up with a solution that will deadlock as a result. I don't think a deadlock is possible in my solution as-is unless I overlooked something.

The TCP logic is something you have to really spend time on understanding as well. You have to maintain the current operation's byte count to know if you need to send/recv more data or if you can move on. For sending data, you want to use some type of list for the data you wish to send out as trying to use one big buffer can get annoying to manage.

You might just want to start simpler with a Console application that uses basic send/recv operations with only one connection first to make sure you have the concepts down.

Quote:And before anyone asks, yes, the packets are identical in both versions, because I am calling the same functions... just in the first example, from a putton click, second example, from inside a thread.


Have you taken into consideration 'time'. I.e., if you were sending packets too fast in the automatic version, that could trigger a disconnect where as using buttons introduces a user delay that might be enough not to cause such a DC. In that case the packets would still be the same, just the way the server processes them changes. You have to keep things like that in mind as well when working on this stuff.

This topic is closed to new replies.

Advertisement