0

I am using a system.Timers Timer in Asynchronous Socket listener console app. I want the timer to run every 40 seconds and check connection status of client devices.

Once the timer starts Server socket is not listening any more(not waiting for more connections) and only data can be received all the time by 1st device that was connected. When I removed the device from network and again connected there is nothing happening. No other function is running. Here is the code for timer-

IPStatusTimer = new System.Timers.Timer(35000);
        IPStatusTimer.Elapsed += IPStatusTimer_Elapsed;
        IPStatusTimer.AutoReset = true;
        IPStatusTimer.Enabled = true;

  private static void IPStatusTimer_Elapsed(object sender, System.Timers.ElapsedEventArgs e)
    {
        Console.WriteLine("Inside Timer");
        if (connectedClient > 0)
        {
            foreach (KeyValuePair<string, int> i in IPStatus)
            {
                if (i.Value == 1)
                {
                    IPStatus[i.Key] = 0;
                }
                else if (i.Value == 0)
                {
                    Decode dr = new Decode();
                    string m = dr.offlineMessage();
                    if (Clients.Count > 0)
                    {
                        foreach (KeyValuePair<int, StateObject> c in Clients)
                        {
                            if (((IPEndPoint)c.Value.workSocket.RemoteEndPoint).Address.ToString() == i.Key)
                            {
                                Clients.Remove(c.Key);
                            }
                        }
                        SendMessage(i.Key, m);
                    }
                }
            }
        }
    }

The timer should not affect my other functions. It should quietly run in background till the app is working.

EDIT: (In above code)Added if CLients.Count>0 then only will remove item from CLients So that there wont be any exception there.

Whenever there is a connection I am adding the Stateobject to dictionary so that I can broadcast data to all the clients when require.

    public static void AcceptCallback(IAsyncResult ar)
    {
        connectedClient++;
        // Signal the main thread to continue.  
        allDone.Set();
        // Get the socket that handles the client request.  
        Socket listener = (Socket)ar.AsyncState;
        Socket handler = listener.EndAccept(ar);
        string ip = ((IPEndPoint)handler.RemoteEndPoint).Address.ToString();
        Console.WriteLine("ip   " + ip);
        // Create the state object.  
        StateObject state = new StateObject();
        state.buffer = new byte[StateObject.BufferSize];
        Clients.Add(connectedClient, state);
        if (IPStatus.ContainsKey(ip))
            IPStatus[ip] = 1;
        else
        IPStatus.Add(ip, 1);
        Console.WriteLine(" Total Connected client : " + connectedClient);
        state.workSocket = handler;           
        handler.BeginReceive(state.buffer, 0, StateObject.BufferSize, 0,
            new AsyncCallback(ReadCallback), state);
    }       

So as you can see.. If a client is back in network and server is always in listening mode it should accept connection and state object should be added to Client(dictionary object) again. It should not be a problem. And as soon as client gets connected I am giving IPStatus(dictionary object) a value 1 as it means client is online.

Then in my readCallBack(), whenever I receive data from any client, then in my dictionary , I am again setting its value to 1 in IPStatus.

 public static void ReadCallback(IAsyncResult ar)
    {
        string content = string.Empty;

        // Retrieve the state object and the handler socket  
        // from the asynchronous state object.  
        StateObject state = (StateObject)ar.AsyncState;
        Socket handler = state.workSocket;
        // Read data from the client socket.   
        int bytesRead = handler.EndReceive(ar);
        int i = 0;
        string ip = ((IPEndPoint)handler.RemoteEndPoint).Address.ToString();
        if (IPStatus.ContainsKey(ip))
        {
            IPStatus[ip] = 1;
            }
}

And when Timer runs , If Value of any IP is 1 I am setting it to 0. So that next time if timer runs and it gets value 0 that means it had not received any data from client in last 35 seconds which means client is not online anymore. ReadCallback and all is happening using StateObject not with Clients(dictinary Object). So if I am removing any item from Clients, I am adding to it again when Any new connection starts.

I hope my scenario is getting a bit more clear to you guys.

Usha phulwani
  • 184
  • 4
  • 22
  • a timer is working as per expectation it may be possible you haven't handled the exception so your code may throw the exception and stop your app. – Dhaval Patel Nov 20 '18 at 12:21
  • Looks to me as if you remove all your clients. Always. 1st run sets IPStatus to 0, second run removes clients. Done - your app is dead. (even if not regarding possible and probable exceptions) – Fildor Nov 20 '18 at 12:33
  • 1
    And I bet the exception is "InvalidOperationException: Collection was modified" and it is thrown when the code removes elements from Clients collection while iterating over it. – rs232 Nov 20 '18 at 12:34
  • @Fildor I am sorry.. I didnt explained it clear in question. But I am editing now... – Usha phulwani Nov 21 '18 at 00:49
  • @rs232 I hope I am a bit more clear now. Client is broadcasting message every 30 seconds. So if I reaceive any data from client in 30 seconds that means client is online and I am setting its value to 1. But if there is no data from the client in last 30 seconds(I am cheking it every 35 seconds giving a window of 5 seconds just in case) that means the value I have set for that client will not be modified(if I set it to 0 its 0). And 0 means ReadCallback have not got any data from that client. And no data means client offline. SO i am deleting its record from Clients. – Usha phulwani Nov 21 '18 at 01:30
  • @DhavalPatel I have made edits. Please look into it and let me know what is wrong now. As I am not getting any exceptions. Only Issue I can see is server is not listening anymore to incoming connections after timer started. Every other thing is working as required. If there would be any exceptions My app would have stopped not running continuously and receiving data from conencted clients. – Usha phulwani Nov 21 '18 at 01:36
  • I am sorry guys... It looks like you guys are right. Timer is working fine. The issue is with Socket server only. Its not getting connected to other clients after some time. I am trying to find out in which part of code its throwing error. Will ask again if needed. – Usha phulwani Nov 21 '18 at 02:55
  • @Ushaphulwani Thank you for the detailed explanation. Unfortunately, that "if (Clients.Count > 0)" check is redundant and will not fix anything. Exception occurs when you do "Clients.Remove(c.Key);" not because the Clients collection is empty, but because you're inside foreach block. Please refer to https://stackoverflow.com/questions/1154350/system-invalidoperationexception-collection-was-modified for more information. – rs232 Nov 21 '18 at 06:38
  • @rs232 Thank you.. I got where I was mistaking... I will resolve it now and update here as soon as issue resolves. Thanks again – Usha phulwani Nov 21 '18 at 06:40

1 Answers1

0

Thanks to @rs232... to point out the cause of exception. It was dictionary object that was throwing error. So used lock(object) before read and write operation on dictionary.

lock (Clients)
                    {
                        if (Clients.Count > 0)
                        {
                            foreach (KeyValuePair<string, StateObject> c in Clients)
                            {
                                if (c.Value.workSocket == handler)
                                {
                                    Clients.Remove(c.Key);
                                    Console.WriteLine("Client automatically disconnected");
                                    Decode dr = new Decode();
                                    string m = dr.offlineMessage();
                                    SendMessage(ip, m);
                                    break;
                                }
                            }
                        }
                    }

And for anyone else who comes on this page thinking about timer issue. The code above(in the question) is perfectly fine, working as required. You can take a reference from that.

Usha phulwani
  • 184
  • 4
  • 22
  • Unfortunately, that code is not fine by any means :( It might somehow work for some reason (I actually doubt it), but it shouldnt' ever be used as a reference: you'll get sudden exceptions when processing disconnected clients. – rs232 Dec 10 '18 at 15:00
  • @rs232My code is just listening to all the incoming connections on port 1200. And when my code wants to send data its job is to first check if that client is still broadcasting data or not. and then only process to send. So there is no exception caused till now from that date. I run that program every day for 12 hours continuously.. – Usha phulwani Dec 11 '18 at 11:04
  • @rs232 yes.. you are right... if The exception will be thrown then it will be catched. And that also mean that I wont receive any data for next 30 seconds(i.e; default time of broadcasting by client machine) and so I will remove the socket from dictionary. next time no exception for that as it will not try to send. – Usha phulwani Dec 12 '18 at 10:44