C#/.Net Appart Thread met socket stoppen

[roy-t]

Ik probeer op dit moment om een programma te schrijven dat berichten ontvangt, daarvoor gebruik ik heel simpel sockets in C#, ik gebruik hier een appart thread voor zodat alles lekker asynchroon kan. Ik heb er expliciet niet voor gekozen om de async functies te gebruiken maar gewoon een while(true) loopje in dus een appart thread, dit om het allemaal zo simpel mogelijk te houden.

De code hieronder is zo als jullie zien vrij basic.

C#:

public class Server
    {
        private Thread thread;

        public void Start()
        {
            thread = new Thread(new ThreadStart(ServerLoop));
            thread.Start();
        }

        private void ServerLoop()
        {
            TcpListener listener = new TcpListener(1555);
            listener.Start(100);
            while (true)
            {
                Socket client = listener.AcceptSocket(); //boosdoener
                //doe spul
            }
        }

        public void Stop()
        {
            thread.Abort();
        }
    }

Het probleem zit hem erin dat ik dit thread niet kan stoppen. Als ik via een andere class Stop() aanroep gaat dat vrolijk goed zonder foutmeldingen, maar toch blijft het process lopen, zelfs als ik Application.Exit() aanroep. Dit is erg vervelend omdat je als je het programma dan opnieuw start je een error krijgt mbt al gebinde porten. (Ook is het gewoon zonde natuurlijk).

Nu vraag ik me af hoe ik er voor kan zorgen dat het thread echt stopt. Op dit moment stopt het pas nadat er nog 1 keer een socket geaccept wordt. Het is dan ook de listener.AcceptSocket() die het thread zelfs in zoverre blocked dat hij niet geabort wordt, wat vreemd is want de Stop methode wordt door een ander thread aangeroepen (het main thread), dat na het aanroepen van Stop() ook gewoon vrolijk verder gaat.

Ik heb gelezen over manual reset events en flags in de while loop, maar dat lost zover ik het snap dit probleem absoluut niet op. Zelf een bericht naar jezelf sturen zou het oplossen maar dat is natuurlijk ook foei lelijk.

Wie o wie heeft hier een schone oplossing voor.

[doctormetal]

Dit is wat je moet hebben: http://msdn.microsoft.com/en-us/library/5b50fdsz.aspx

[whoami]

Gebruik nooit Thread.Abort om je thread proper te laten stoppen.
Als je dat doet wordt er nl. een ThreadAbortException gegooid, die je thread onmiddellijk doet stoppen, no matter what hij bezig is.

Het is beter om dus zoiets te doen:

code:

public class Server
{
    public void Start()
    {
           Thread t = new Thread(new ThreadStart(ServerLoop));
           t.Start();
    }

    private bool _canceled = false;

    private void ServerLoop()
    {
            TcpListener l = new TcpListener();

            l.Start();

            while( ! _canceled )
            {
                 // doe uw ding
            }

            l.Stop();
    }

    public void Cancel()
    {
         _canceled = true;
    }
}

En zorg er ook voor dat je je listener stopt.

[ Voor 10% gewijzigd door whoami op 19-08-2009 14:11 ]

[Sebazzz]

@whoami: Uhm, ik zou voor de zekerheid nog een lock toevoegen, je weet maar nooit

C#:

public class Server
{
    public void Start()
    {
           Thread t = new Thread(new ThreadStart(ServerLoop));
           t.Start();
    }

    private object _syncRoot = new object();
    private bool _cancellationRequested = false;

    private void ServerLoop()
    {
            while( true ) {
                lock (_syncRoot) {
                     if ( _cancellationRequested ) {
                           break;
                     }
                }
                 // doe uw ding
            }
    }

    public void Cancel()
    {
        lock (_syncRoot) {
             _cancellationRequested = true;
        }
    }
}

Het probleem met Thread.Abort is inderdaad dat die exception ook in unmanaged code opgegooid kan worden, en je kan nooit weten hoe die code daarop reageert. Wat zou er gebeuren als je met Direct3D bezich ben, en daar wordt een exception in opgegooid? Of bij een File operatie? Of in dit geval bij networking?

[ Voor 16% gewijzigd door Sebazzz op 19-08-2009 14:30 . Reden: managed code -> unmanaged code ]

[whoami]

Hmm ....
Denk je dat het noodzakelijk is ? Ik denk niet dat er hier echte race condities kunnen optreden in dit geval, aangezien je niets anders doet dan gewoon cancel op true zetten, het is niet dat er verschillende threads tegelijk die status zitten te pollen en te veranderen.

Trouwens, je kan ook gewoon direct locken op _cancellationRequested ook.

[roy-t]

@ Whoami en Sebazzz, dit lost het probleem toch absoluut niet op, het thread blijft nogsteeds hangen op listener.AcceptSocket() en komt pas bij een volgende connectie weer uit bij het evalueren van de while loop.

Verder weet ik niet zeker of de lock ook nodig is, maar het is natuurlijk nooit onverstandig.

@doctormetal, dat is al wat ik doe en werkt dus niet .

[doctormetal]

Sebazzz schreef op woensdag 19 augustus 2009 @ 14:13:
Het probleem met Thread.Abort is inderdaad dat die exception ook in managed code opgegooid kan worden, en je kan nooit weten hoe die code daarop reageert. Wat zou er gebeuren als je met Direct3D bezich ben, en daar wordt een exception in opgegooid? Of bij een File operatie? Of in dit geval bij networking?

Dit geldt voor elke mogelijke exceptie. Die kunnen op elk moment komen. Je code moet gewoon goed om kunnen gaan daarmee.

Het voordeel van ThreadAbort is dat deze ook blocking calls kan onderbreken.
De methode van whoami kan dit niet en zal dus tot gevolg hebben dat het stoppen van de thread erg lang kan duren. Die methode heeft tevens als nadeel dat je continue moet gaan kijken of je wel of niet moet stoppen.

[Sebazzz]

roy-t schreef op woensdag 19 augustus 2009 @ 14:24:
@ Whoami en Sebazzz, dit lost het probleem toch absoluut niet op, het thread blijft nogsteeds hangen op listener.AcceptSocket() en komt pas bij een volgende connectie weer uit bij het evalueren van de while loop.

Je weet hoe IT'ers zijn hè, is er iets niet perfect, er gelijk bovenop.

Heb je al naar TcpListener.Pending( void ) gekeken

doctormetal schreef op woensdag 19 augustus 2009 @ 14:26:
[...]

Dit geldt voor elke mogelijke exceptie. Die kunnen op elk moment komen. Je code moet gewoon goed om kunnen gaan daarmee.

Uhm, een .NET exception wordt in managed code opgegooid en in managed code opgevangen of niet opgevangen. Een ThreadAbortException wordt in managed code opgegooid, maar de kans is er dat hij terecht komt in unmanaged code. Dat wil je niet.

[ Voor 30% gewijzigd door Sebazzz op 19-08-2009 14:29 ]

[Mental]

private void ServerLoop()
{
TcpListener listener = new TcpListener(1555);
listener.Start(100);
while (true)
{
Socket client = listener.AcceptSocket(); //boosdoener
//doe spul
}
}

Vrij vertaald:

- Maak listeneraan
[start loop]
- Maak een socket aan
[stop loop]

Kan je het aanmaken van een socket niet veel beter met een event doen?
Lijkt me ook meer de bedoeling dan hoe je het nu doet.

[whoami]

doctormetal schreef op woensdag 19 augustus 2009 @ 14:26:
[...]

Dit geldt voor elke mogelijke exceptie. Die kunnen op elk moment komen. Je code moet gewoon goed om kunnen gaan daarmee.

Het voordeel van ThreadAbort is dat deze ook blocking calls kan onderbreken.
De methode van whoami kan dit niet en zal dus tot gevolg hebben dat het stoppen van de thread erg lang kan duren. Die methode heeft tevens als nadeel dat je continue moet gaan kijken of je wel of niet moet stoppen.

Ha, maar als je ThreadAbort gooit, dan kan je nooit je Thread mooi afsluiten.
Thread.Abort gebruiken om een thread te stoppen is iets wat je ten alle tijde moet vermijden, aangezien je nooit je huidige bewerking proper kunt afmaken, resources vrijgeven, etc...

roy-t: dat is logisch ook aangezien de AcceptSocket method een blocking method is. Die method zal dus wachten tot wanneer er iets beschikbaar is om in te lezen.
Je kan de Pending method gebruiken om te zien of er idd iets uit te lezen valt.

[bigbeng]

Volgens de documentatie is dit precies wat je zou mogen verwachten. AcceptSocket is namelijk een blocking method. In diezelfde documentatie staat ook dat je ipv een directe call op AcceptSocket zou kunnen kijken of er pending sockets zijn om te accepteren. Deze pagina is er voor jou:
http://msdn.microsoft.com...istener.acceptsocket.aspx

damn... toch jammer als je tijdens het schrijven van een reply al weet dat iemand eerder was. Maar goed, mijn post heeft als kleine extra een link naar de documentatie

[H!GHGuY]

whoami schreef op woensdag 19 augustus 2009 @ 14:16:
Trouwens, je kan ook gewoon direct locken op _cancellationRequested ook.

Ben je dit zeker? Volgens mij heeft Monitor een object nodig en geen struct-type.
In het ergste geval wordt de bool geboxed en wordt op dit tijdelijk geboxed object gelocked.

[whoami]

Nou doe je mij twijfelen.
Idd.

Use Monitor to lock objects (that is, reference types), not value types. When you pass a value type variable to Enter, it is boxed as an object. If you pass the same variable to Enter again, it is boxed as a separate object, and the thread does not block.

[ Voor 84% gewijzigd door whoami op 19-08-2009 14:47 ]

[Sebazzz]

Volgens mij is het zowieso good practice om één object als uitverkorene te gebruiken voor het locken en niet iets anders waarmee je dingen doet, eventueel assignements naar doet.

[ Voor 26% gewijzigd door Sebazzz op 19-08-2009 14:48 ]

[iWodan]

roy-t schreef op woensdag 19 augustus 2009 @ 14:24:
@ Whoami en Sebazzz, dit lost het probleem toch absoluut niet op, het thread blijft nogsteeds hangen op listener.AcceptSocket() en komt pas bij een volgende connectie weer uit bij het evalueren van de while loop.

Verder weet ik niet zeker of de lock ook nodig is, maar het is natuurlijk nooit onverstandig.

@doctormetal, dat is al wat ik doe en werkt dus niet .

probeer eens om die listener op class-level te zetten, en in uw stop-method, de 'listener.stop()' op te roepen ?
dit zou moeten werken.

Grtz.

[Sebazzz]

iWodan schreef op woensdag 19 augustus 2009 @ 14:55:
[...]


probeer eens om die listener op class-level te zetten, en in uw stop-method, de 'listener.stop()' op te roepen ?
dit zou moeten werken.

quote: MSDN - TcpListener - Thread Safety

Thread Safety
Any public static (Shared in Visual Basic) members of this type are thread safe. Any instance members are not guaranteed to be thread safe.

Toch maar niet

[roy-t]

bigbeng schreef op woensdag 19 augustus 2009 @ 14:34:
Volgens de documentatie is dit precies wat je zou mogen verwachten. AcceptSocket is namelijk een blocking method. In diezelfde documentatie staat ook dat je ipv een directe call op AcceptSocket zou kunnen kijken of er pending sockets zijn om te accepteren. Deze pagina is er voor jou:
http://msdn.microsoft.com...istener.acceptsocket.aspx

damn... toch jammer als je tijdens het schrijven van een reply al weet dat iemand eerder was. Maar goed, mijn post heeft als kleine extra een link naar de documentatie

Ja een blocking call, een Thread.Abort kan volgens de documentatie ook blocking calls stoppen omdat dit vanuit een ander thread komt, alleen bij sockets blijkt dit dus niet goed te werken, ik heb natuurlijk al de documentatie nagelezen, hoewel dat misschien beter had vermeld moeten worden in de TS .

@Sebazzz:
Oeh ik ga zeker eens kijken naar TcpListener.Pending( void ), bedankt voor de tip, dat ziet er uit als een winaar!

@l4Mor, dat kan via Asynchrone socket methodes (BeginAccept e.d.) maar hoewel ik er een testcaseje mee geschreven hebt vind ik dat dit dingen toch nodeloos ingewikkeld maakt voor zo'n simpel systeempje als dat ik nu schrijf (staat ook in de TS trouwens!).

@iWodan/Sebazzz,

Hmm dat kan dus helaas niet door wat Sebazzz gequote heeft, een singleton met statics is ook helaas geen optie.

Anyway ik ga is naar die Pending() methode kijken, kan volgende week pas echt weer werken aan het programma, maar dit ziet er wel hoopvol uit (en kon het niet uitstaan dat ik het niet goed kreeg).

Wel weer een mysterie waarom Thread.Abort wel blocking calls kan stoppen maar dan net weer niet deze, misschien omdat het in een methode zit van de class ipv van buiten af, maar dat zou niet mogen uitmaken omdat in mijn geval de main thread en niet het blokkend thread de Stop() methode van Server aanroept.

[Haan]

Onderaan de Thread.Abort() documentatie staat ook nog een stukje user comment waarom je deze methode bij voorkeur niet gebruikt.

Zie ook: http://msmvps.com/blogs/p...rly-designed-program.aspx

Met name het argument dat je met Thread.Abort eigenlijk een exception gebruikt om de flow van je programma te beïnvloeden, terwijl exceptions daar niet voor bedoeld zijn, kan ik me in vinden.

[Verwijderd]

Wat je ook kan doen is je socket non-blocking maken. Wat er dan gebeurt is dat de socket een read doet, alles wat gelezen wordt teruggeeft (zoals normaal), maar niet blockt als er niets is. Dan gaat je thread door. Wat je dan wel moet doen is iets van een sleep inbouwen, anders gaat je thread heel veel CPU vreten gok ik.

Volgens mij heeft C# op het socket object gewoon een property genaamd 'Blocking', die je dan op false moet zetten. Wellicht dat dat een andere oplossing voor je probleem is?

[whoami]

Verwijderd schreef op donderdag 20 augustus 2009 @ 13:40:
Volgens mij heeft C# op het socket object gewoon een property genaamd 'Blocking', die je dan op false moet zetten. Wellicht dat dat een andere oplossing voor je probleem is?

Aannames zijn fataal.

Maar, er zijn natuurlijk wel de non-blocking methods zoals BeginAcceptSocket / EndAcceptSocket bv.

Maar in dit geval zou ik ook gewoon de 'Pending' method uitlezen.

[roy-t]

Nahh ik wordt even gek, kon vandaag weer verder met die code en netjes even pending gebruiken, lijkt allemaal goed te werken totdat ik nog eens even nakijk of dat thread nu echt er mee ophoud.

Blijkt dat op een of andere manier, zelfs met een lock op een leeg syncroot object cancel nooit true wordt in het thread van de while loop, ondanks dat het wel true wordt in de stop methode .

C#:

public class Server
    {
        public delegate void received(string message);
        public event received OnReceived;
        private Thread thread;
        private int port;
        private bool cancel;
        private object syncroot = new object();

        public void Start(int port)
        {
            this.port = port;            
            thread = new Thread(new ThreadStart(ServerLoop));
            cancel = false;
            thread.Start();
        }

        private void ServerLoop()
        {
            try
            {
                TcpListener listener = new TcpListener(port);
                listener.Start(100);
                while (true)
                {
                    lock (syncroot)
                    {
                        if (cancel)
                        {
                            break;
                        }
                    }

                    if (listener.Pending())
                    {
                        Socket client = listener.AcceptSocket();
                        byte[] buffer = new byte[256];
                        client.Receive(buffer);
                        if (OnReceived != null)
                        {
                            OnReceived(Encoding.UTF8.GetString(buffer));
                        }
                    }
                    else
                    {
                        Thread.Sleep(500);
                    }
                }
                listener.Stop();
            }
            catch (Exception e)
            {
                MessageBox.Show("Error: " + e.Message);
                Application.Exit();
            }
        }        

        public void Stop()
        {
            lock (syncroot)
            {
                cancel = true;
            }
        }
    }

De server wordt natuurlijk heel simpel zo aangemaakt.

C#:

            server = new Server();            
            server.Start(1550);

Eerst heb ik de stop methode gewoon aangeroepen, maar nu roep ik die met een invoke zelfs aan op dat thread.

C#:

private delegate void StopThreadDelegate();
        private void StopThread()
        {
            server.Stop();
        }        

        private void frmComputerManager_FormClosed(object sender, FormClosedEventArgs e)
        {
            Invoke(new StopThreadDelegate(StopThread), null);            
            Application.Exit();
        }

Zoals verteld zie ik tijdens het debuggen netjes de stop methode aangeroepen worden en canceled op true gezet worden, maar zodra ik naar het volgende breakpoint ga (op de while(true) staat canceled weer op false. Mis ik nu iets heel doms of is dit wel een erg eigenaardig threading probleem? Alles is netjes gelocked en dat zou niet eens nodig moeten zijn omdat er maar 1 methode die is die maar 1 ding kan wegschrijven (namelijk canceled = true) verder wordt er alleen maar gelezen. (Er is maar 1 iemand dat stop aanroept en dat gebeurt dus ook echt maar 1x).

Mehh ik ga nog maar eens even goed kijken, maar ik zie echt iets over het hoofd

* roy-t is nog een beetje een noobje met threads.

[Woy]

ik zou sowieso je cancel bool eens als volatile markeren, anders kan het zijn dat hij lokaal gecached word, en dus idd nooit goed geupdate word. ( http://msdn.microsoft.com/en-us/library/x13ttww7(VS.71).aspx )

Volgens mij is de syncRoot overigens niet perse nodig, aangezien je alleen een write op cancel doet, zonder eerst een controle o.i.d. Het beschrijven van een bool is volgens mij wel een atomic operation. Maar dat heeft verder niks met het probleem te maken.

Ik zou overigens toch eens naar de asynchrone functies kijken, zo ingewikkeld is dat ook weer niet, en je hebt ieder geval niet het probleem dat je moet sleepen om een busy-wait te voorkomen.

[ Voor 66% gewijzigd door Woy op 24-08-2009 10:48 ]

[roy-t]

Ik heb de bool als volatile gemarkeerd maar dit haalt helaas niet uit, het is erg gek dat het thread gewoon weigert om nieuwe waarden te zien.

Ook als ik van cancel een private static volatile bool maak veranderd dit niets (dit om eventuele andere threading problemen te voorkomen). Ook als ik er een methode van maak om het op te zoeken veranderd er niets, en ook als ik de bool in een heel andere class zet die in een ander thread draait krijg ik die niet goed te zien in het server thread, ik zie echt iets doms over het hoofd lijkt het wel. Overstappen op async server opties zou kunnen, maar dan moet ik wel veel dingen weer omgooien, misschien is het op dit moment wel de betere optie want nu zit ik echt nutteloos te kloten met threads.

[Woy]

roy-t schreef op maandag 24 augustus 2009 @ 10:50:
Ik heb de bool als volatile gemarkeerd maar dit haalt helaas niet uit, het is erg gek dat het thread gewoon weigert om nieuwe waarden te zien.

volatile zorgt er voor dat een thread altijd gegarandeerd de nieuwe waarde leest, anders zou hij de waarde bijvoorbeeld in zijn processor cache kunnen hebben en niet updaten. Ik denk dat je toch een ander probleem ergens hebt.

Ook als ik van cancel een private static volatile bool maak veranderd dit niets (dit om eventuele andere threading problemen te voorkomen). Ook als ik er een methode van maak om het op te zoeken veranderd er niets, en ook als ik de bool in een heel andere class zet die in een ander thread draait krijg ik die niet goed te zien in het server thread, ik zie echt iets doms over het hoofd lijkt het wel. Overstappen op async server opties zou kunnen, maar dan moet ik wel veel dingen weer omgooien, misschien is het op dit moment wel de betere optie want nu zit ik echt nutteloos te kloten met threads.

waarom heb je de cancel static gemaakt? Dat kan natuurlijk ook problemen opleveren als je straks meerdere instanties van je Server wil hebben.

iets als de volgende code zou gewoon moeten werken

C#:

public class ThreadedServer
{
    private volatile bool cancel = false;
    
    public ThreadedServer()
    {
        Thread thread = new Thread(Run);
        thread.Start();
    }

    private void Run()
    {
        while(!cancel)
        {
            //Doe je ding hier
        }
    }

    public void Stop()
    {
        cancel = true;
    }
}

[Hydra]

roy-t schreef op maandag 24 augustus 2009 @ 10:50:
Ik heb de bool als volatile gemarkeerd maar dit haalt helaas niet uit, het is erg gek dat het thread gewoon weigert om nieuwe waarden te zien.

Ga eens debuggen dan, want dat kan in princiepe gewoon niet.

[roy-t]

Zoals ik aangaf heb ik redelijk door code lopen stappen . Heb net alles omgezet naar een asynchrone variant nu ik er toch achterkwam waardoor het fout ging, ergens anders in het programma werd nog een 2e server aangemaakt doh. En die liep natuurlijk gewoon door. Iig heb ik nu een nettere asynchrone variant en het viel inderdaad best mee om dat even in elkaar te zetten, kan me nog herinneren van vroeger een tijdje terug dat dat best een klus was (toen nog een artikeltje over geschreven) maar met oude artikel erbij in een halfuurtje in elkaar gedraait.

Het probleem was toch mijn domme schuld (doh alweer!). Maar iig heb ik het nu netter opgelost. Ik heb hieronder de code nog maar even geplakt mocht iemand er nog wat aan hebben.

* roy-t is echt absoluut niet helder, griep nog in mijn kop

C#:

public class MServer
    {
        public delegate void received(string message);
        public event received OnReceived;

        private ManualResetEvent allDone = new ManualResetEvent(false);
        private Thread thread;        
        private bool running = true;        
        private int port;

        public void Start(int port)
        {
            this.port = port;                        
            thread = new Thread(new ThreadStart(RunServer));
            thread.Start();
        }

        private void RunServer()
        {
            Socket listener = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
            listener.Bind(new IPEndPoint(IPAddress.Loopback, port));
            while (running)
            {
                allDone.Reset();
                listener.Listen(100);
                listener.BeginAccept(Accept, listener);
                allDone.WaitOne();
            }
        }

        private void Accept(IAsyncResult result)
        {
            BufferSocket bs = new BufferSocket(result);
            bs.socket.BeginReceive(bs.buffer, 0, 256, SocketFlags.None, Receive, bs);
        }

        private void Receive(IAsyncResult result)
        {
            BufferSocket bs = (BufferSocket)result.AsyncState;
            int read = bs.socket.EndReceive(result);
            if (read != 0)
            {
                if (OnReceived != null)
                {
                    OnReceived(Encoding.UTF8.GetString(bs.buffer));
                }
            }
            allDone.Set();
        }

        public void Stop()
        {
            running = false;
            allDone.Set();
        }


        private struct BufferSocket
        {
            public Socket socket;
            public byte[] buffer;

            public BufferSocket(IAsyncResult result)
            {
                socket = ((Socket)result.AsyncState).EndAccept(result);
                buffer = new byte[256];
            }
        }
    }