Hello All.  I'm trying to open multiple TcpListeners on a Cp4 using Simpl# program (not a library).
I have managed to be able to open 1, connect and send/receive messages from the first server (port 1234). But not the second (port 5678)

using System;
using System.Text;
using Crestron.SimplSharp;                          	// For Basic SIMPL# Classes
using Crestron.SimplSharpPro;                       	// For Basic SIMPL#Pro classes
using Serilog;                                          // For Logging
using Serilog.Core;                                     // For switching Logging levels
using Serilog.Events;                                   // For Logging levels
using Serilog.Enrichers.WithSimpleCaller;
using System.Net.Sockets;
using System.Collections.Generic;


namespace CrestronProgram
{
    public class ControlSystem : CrestronControlSystem
    {
        ///
        /// ControlSystem Constructor. Starting point for the SIMPL#Pro program.
        /// Use the constructor to:
        /// * Initialize the maximum number of threads (max = 400)
        /// * Register devices
        /// * Register event handlers
        /// * Add Console Commands
        /// 
        /// Please be aware that the constructor needs to exit quickly; if it doesn't
        /// exit in time, the SIMPL#Pro program will exit.
        /// 
        /// You cannot send / receive data in the constructor
        /// 
        public ControlSystem()
            : base()
        {
            try
            {
                Crestron.SimplSharpPro.CrestronThread.Thread.MaxNumberOfUserThreads = 50;

                //Subscribe to the controller events (System, Program, and Ethernet)
                CrestronEnvironment.SystemEventHandler += new SystemEventHandler(_ControllerSystemEventHandler);
                CrestronEnvironment.ProgramStatusEventHandler += new ProgramStatusEventHandler(_ControllerProgramEventHandler);
                CrestronEnvironment.EthernetEventHandler += new EthernetEventHandler(_ControllerEthernetEventHandler);
            }
            catch (Exception e)
            {
                ErrorLog.Error("Error in the constructor: {0}", e.Message);
            }
        }

        ///
        /// InitializeSystem - this method gets called after the constructor 
        /// has finished. 
        /// 
        /// Use InitializeSystem to:
        /// * Start threads
        /// * Configure ports, such as serial and verisports
        /// * Start and initialize socket connections
        /// Send initial device configurations
        /// 
        /// Please be aware that InitializeSystem needs to exit quickly also; 
        /// if it doesn't exit in time, the SIMPL#Pro program will exit.
        /// 
        public override void InitializeSystem()
        {
            try
            {
                System.Threading.Thread StartupThread = new System.Threading.Thread(() => Startup());
                StartupThread.Name = "Startup";
                StartupThread.Start();
            }
            catch (Exception e)
            {
                ErrorLog.Error("Error in InitializeSystem: {0}", e.Message);
            }
        }

        ///
        /// Event Handler for Ethernet events: Link Up and Link Down. 
        /// Use these events to close / re-open sockets, etc. 
        /// 
        ///This parameter holds the values 
        /// such as whether it's a Link Up or Link Down event. It will also indicate 
        /// wich Ethernet adapter this event belongs to.
        /// 
        void _ControllerEthernetEventHandler(EthernetEventArgs ethernetEventArgs)
        {
            switch (ethernetEventArgs.EthernetEventType)
            {//Determine the event type Link Up or Link Down
                case (eEthernetEventType.LinkDown):
                    //Next need to determine which adapter the event is for. 
                    //LAN is the adapter is the port connected to external networks.
                    if (ethernetEventArgs.EthernetAdapter == EthernetAdapterType.EthernetLANAdapter)
                    {
                        Log.Error($"{ethernetEventArgs.EthernetAdapter} Link is Down.");
                    }
                    break;
                case (eEthernetEventType.LinkUp):
                    if (ethernetEventArgs.EthernetAdapter == EthernetAdapterType.EthernetLANAdapter)
                    {
                        Log.Error("{ethernetEventArgs.EthernetAdapter} Link is Up.");
                    }
                    break;
            }
        }

        ///
        /// Event Handler for Programmatic events: Stop, Pause, Resume.
        /// Use this event to clean up when a program is stopping, pausing, and resuming.
        /// This event only applies to this SIMPL#Pro program, it doesn't receive events
        /// for other programs stopping
        /// 
        ///
        void _ControllerProgramEventHandler(eProgramStatusEventType programStatusEventType)
        {
            switch (programStatusEventType)
            {
                case (eProgramStatusEventType.Paused):
                    //The program has been paused.  Pause all user threads/timers as needed.
                    Log.Information("Program has been paused.");
                    break;
                case (eProgramStatusEventType.Resumed):
                    //The program has been resumed. Resume all the user threads/timers as needed.
                    Log.Information("Program has been paused.");
                    break;
                case (eProgramStatusEventType.Stopping):
                    //The program has been stopped.
                    //Close all threads. 
                    //Shutdown all Client/Servers in the system.
                    //General cleanup.
                    //Unsubscribe to all System Monitor events
                    break;
            }

        }

        ///
        /// Event Handler for system events, Disk Inserted/Ejected, and Reboot
        /// Use this event to clean up when someone types in reboot, or when your SD /USB
        /// removable media is ejected / re-inserted.
        /// 
        ///
        void _ControllerSystemEventHandler(eSystemEventType systemEventType)
        {
            switch (systemEventType)
            {
                case (eSystemEventType.DiskInserted):
                    //Removable media was detected on the system
                    Log.Information("Disk Inserted");
                    break;
                case (eSystemEventType.DiskRemoved):
                    //Removable media was detached from the system
                    Log.Information("Disk Removed");
                    break;
                case (eSystemEventType.Rebooting):
                    //The system is rebooting. 
                    //Very limited time to preform clean up and save any settings to disk.
                    Log.Information("System is Rebooting");
                    break;
            }
        }

        #region Custom Stuff
        List Servers;
        List Clients;

        ///
        /// Log Level
        /// 
        private LoggingLevelSwitch DebugLevel;

        ///
        /// Launched as a separate thread at InitializeSystem
        /// 
        void Startup()
        {
            try
            {
                //configure Serilog
                DebugLevel = new LoggingLevelSwitch();
                DebugLevel.MinimumLevel = LogEventLevel.Verbose;

                Serilog.Log.Logger = new LoggerConfiguration()
                    .MinimumLevel.ControlledBy(DebugLevel)
                    .Enrich.WithSimpleCaller()
                    .WriteTo.DelegateSink(null, LogThis)
                    .CreateLogger();
            }
            catch (Exception ex)
            {
                ErrorLog.Exception("Exception Creating Logger", ex);
                CrestronConsole.PrintLine($"Exception Creating Logger.{ex.ToString()}");
            }

            Log.Verbose("STARTUP");          

            Servers = new List();
            Clients = new List();

            System.Threading.Thread A = new System.Threading.Thread(() => RunServer(1234));
            A.Start();
            System.Threading.Thread B = new System.Threading.Thread(() => RunServer(5678));
            B.Start();

            Log.Verbose("END OF STARTUP");
        }

        ///
        /// send the logevent to console and TcpClients in Clients
        /// 
        ///
        private void LogThis(LogEvent logevent)
        {
            string time = logevent.Timestamp.ToString("MM-dd-yyyy HH:mm:ss.ff");
            string level = logevent.Level.ToString().ToUpper().Substring(0, 4);
            string message = logevent.RenderMessage();
            string caller = logevent.Properties["Caller"].ToString().Trim('"');
            string msg = string.Empty;
            if (logevent.Exception == null)
                msg = $"\r\n[{time} {level}] {caller}\r\n--- {message}\r\n";
            else
                msg = $"\r\n[{time} {level}] {caller}\r\n--- {message}\r\n{logevent.Exception}\r\n{Newtonsoft.Json.JsonConvert.SerializeObject(logevent, Newtonsoft.Json.Formatting.Indented)}\r\n";

            CrestronConsole.PrintLine(msg);
            SendStringToClients(msg);
        }

        
        ///
        /// Create and run a TcpListener. Strings received from the server will be sent to ServerReceivedString.
        /// THis will also popuplate Servers and Clients collection
        /// 
        ///
        public async void RunServer(int port)
        {
            TcpListener listener = new TcpListener(System.Net.IPAddress.Parse("0.0.0.0"), port);
            Servers.Add(listener);

            listener.Start();
            Log.Verbose("Listener started at port {port}", port);
            while (true)
            {
                using (TcpClient tcpClient = await listener.AcceptTcpClientAsync().ConfigureAwait(false))
                {
                    Clients.Add(tcpClient);
                    Log.Verbose("Listener on Port {port} has a client connected from {Ip}", port, ((System.Net.IPEndPoint)tcpClient.Client.RemoteEndPoint).Address);
                    while (tcpClient.Connected)
                    {
                        byte[] rx = new byte[1024];
                        int bytesread = await tcpClient.GetStream().ReadAsync(rx, 0, 1024).ConfigureAwait(false);
                        if (bytesread > 0)
                        {
                            string msg = Encoding.ASCII.GetString(rx);
                            string ip = ((System.Net.IPEndPoint)tcpClient.Client.RemoteEndPoint).Address.ToString();
                            Log.Verbose("Client Received {Ip}:{msg}",ip,msg);
                            ServerReceivedString(msg);
                        }
                        else
                            break;
                        
                    }
                }
            }
        }        

        ///
        /// Process received strings from any server
        /// 
        ///
        void ServerReceivedString(string data)
        {
            Log.Verbose("Server Rx:{data}", data);
            SendStringToClients($"Server has Received {{{data}}}\r\n");
        }
        
        ///
        /// Send string to all clients
        /// 
        ///
        void SendStringToClients(string msg)
        {
            byte[] bytes = Encoding.UTF8.GetBytes(msg);

            foreach(var client in Clients)
            {
                if (client.Connected)
                {
                    try
                    {
                        client.GetStream().WriteAsync(bytes, 0, bytes.Length).ConfigureAwait(false);
                    }
                    catch (Exception ex)
                    {
                        Log.Error(ex, "Could not write to client");
                        client.Close();
                        Clients.Remove(client);
                    }
                }
            }
        }

        #endregion
    }
}

Thread-safety should be a concern here, since these collections (assuming the generic List class here) are not thread-safe collections by default. You could very easily run into issues with your enumerator and collection modification on separate threads; in fact, looking at your enumerator I can see that you try to modify the collection within the foreach loop in your SendStringToClients method in the event of an exception (likely ObjectDisposedException in most cases) which can't work. (See demo: https://dotnetfiddle.net/Sxfdo7)

It seems like you have two servers listening on different ports, but they intermix the data that gets sent to the clients anyways.. Do you actually need to listen on 2 ports here or would 1 port work with a server that accepts multiple clients?

I would advise splitting this up into classes based on the first rule of SOLID design principles and work from there. This particular setup is likely to be fairly unreliable at first glance.. Be cautious about mixing encodings too (i.e. ASCII vs UTF8).

P.S. - Did you create your own non-generic List class? Additionally, when using async methods, you're not awaiting all of your method calls it looks like.

~ Troy

--

On Fri, Dec 24, 2021 at 01:09 PM, Master He wrote:

I think there is exception/error thrown when "public async void RunServer(int port)" is called. But since you use "async void", the exception is discard.

I would wrap the method with try/catch and change it to "async Task" and see what has happened.

Exceptions from async void methods are not discarded, they still get thrown; albeit with different error-handling semantics since there is no Task/Task<T> object to place the exception on. That does bring up another point though, I didn't catch this method signature... It should be public async Task RunServerAsync(int port).

With async void, you can't really catch them in most cases outside of the method that threw the exception because it is raised on the active SynchronizationContext. They're only really intended for callbacks where you can't change the callback signature, but need to utilize async code. If there's any chance of an exception in async void that you can't prevent, I would catch the specific exceptions if possible to provide handling. async Task or Task<T> for literally everything else, then handle the exceptions on the Task/Task<T> object.
 
--