Diskuze: Async TCP server - vyletí využití CPU 60% po odpojení clienta

Erik Šťastný

Člen

Erik Šťastný:10.11.2017 15:53

Zdravíčko, mám TcpListener využivající asynchroní await async metody pro komunikaci s klienty.

Když ale na strašně klienta zavolám TcpClient.close(), tak server nadále funguje, ale vyletí mu využití CPU na 50 - 60%

Na jiném fóru mi bylo řečeno, že nemám používat pro async metody return void, ale fakt mě nenapadá, proč by to způsobovalo to co říkám.

Netuší někdo prosím co s tím?

Třída pro server:

class Server
{
    private TcpListener server;

    public Server(string hostname, int port = 25000)
    {
        server = new TcpListener(IPAddress.Parse(hostname), port);
    }

    public void ServerStart()
    {
        server.Start();
        WaitForClientConnect();
    }

    private async void WaitForClientConnect()
    {
        TcpClient client = await server.AcceptTcpClientAsync();
        Console.WriteLine("The async connection created for: " + ((IPEndPoint)client.Client.RemoteEndPoint).Address.ToString());

        OnClientConnect(client);
    }

    private void OnClientConnect(TcpClient client)
    {
        ClientLowAPI clientReq = new ClientLowAPI(client);

        WaitForClientConnect();
    }
}

Kód pro zachycený jednotlivých klientů:

class ClientLowAPI
{
    private TcpClient client;
    private NetworkStream stream;

    public ClientLowAPI(TcpClient clientConnected)
    {
        client = clientConnected;
        stream = client.GetStream();
        WaitForHeader();
    }

    private async void WaitForHeader()
    {
        byte[] buffer = new byte[4];

        int bytesRead = 0;
        while (bytesRead < 4)
        {
            try
            {
                bytesRead += await stream.ReadAsync(buffer, bytesRead, buffer.Length - bytesRead);
            }
            catch
            {
                stream.Close();
                client.Close();
                return;
            }
        }

        WaitForData(FourBytesToInt(buffer));
    }

    private async void WaitForData(int length)
    {
        byte[] buffer = new byte[length];

        int bytesRead = 0;
        while (bytesRead < length)
        {
            try
            {
                bytesRead += await stream.ReadAsync(buffer, bytesRead, buffer.Length - bytesRead);
            }
            catch
            {
                stream.Close();
                client.Close();
                return;
            }
        }

        ExecuteMessage(buffer);
    }

    private void ExecuteMessage(byte[] binaryData)
    {
        // Do something with message
        WaitForHeader();
    }

    public async void SendMessage(byte[] message)
    {
        byte[] buffer = new byte[message.Length + 4];
        byte[] length = IntToFourBytes(message.Length);

        length.CopyTo(buffer, 0);
        message.CopyTo(buffer, 4);

        try
        {
            await stream.WriteAsync(buffer, 0, buffer.Length);
        }
        catch
        {
            stream.Close();
            client.Close();
            return;
        }
    }

    private int FourBytesToInt(byte[] array)
    {
        int res = 0;
        res += array[0] * 256 * 256 * 256;
        res += array[1] * 256 * 256;
        res += array[2] * 256;
        res += array[3];
        return res;
    }

    private byte[] IntToFourBytes(int intValue)
    {
        byte[] array = new byte[4];
        array[0] = (byte)(intValue >> 24);
        array[1] = (byte)(intValue >> 16);
        array[2] = (byte)(intValue >> 8);
        array[3] = (byte)intValue;
        return array;
    }
}

Editováno 10.11.2017 15:54

Odpovědět

10.11.2017 15:53

Neaktivní uživatel

Člen

Neaktivní uživatel:10.11.2017 17:43

Problém je ten, že když zavřeš spojení na klientovi, tak na serveru ti to při pokusu číst data vrací 0 Při čtení hlavičky máš podmínku bytes < 4, který je tedy pořád true. Takže se pořád dokola snažíš číst a nedostaneš se z while cyklu -> vysoké využití CPU.

Dále tam máš nepřímé rekurze (WaitForClientConnect->OnClientConnect a WaitForHeader->WaitForData->ExecuteMessage). Docela divný návrh. Pokud by server běžel dlouho a připojovalo se hodně lidí, tak by ti časem vyskočila vyjmka StackOverflowException-

Ti druzí mají s void a async pravdu. Protože tam máš hned na začátku pouze WaitForClientConnect, tak hned po zavolání metody se pokračuje dál, tj. nečeká se na připojení. Když změníš async void na async Task a při volání použiješ await, tak se bude čekat. Takže buď používej správně async/await nebo nepoužívej vůbec async metody.

Malý tip na konec. Zkus zvážit použití BinaryReaderu a BinaryWriteru, ať se nemusíš starat o práci s bytama.

Editováno 10.11.2017 17:44

Akceptované řešení
+20 Zkušeností
+2,50 Kč

Nahoru Odpovědět

10.11.2017 17:43

Neaktivní uživatelský účet

Erik Šťastný

Člen

Erik Šťastný:10.11.2017 17:47

Když bych nepoužíval async/await, tak asi vytvářím pro každého uživatele nový Task za což jsem dostal vyhubováno na jiném fórku ještě víc

Jinak k tomu prvnímu mám pocit, že když ten connection uzavřu, tak by to mělo skočit při pokusu o čtení do vyjímky, ne? Ale určitě to otestuju až se k tomu dostanu.

A s tou rekurzí si bohužel nevím rady jak ten návrh udělat jinak jsem bohužel amatér zatím

Nahoru Odpovědět

10.11.2017 17:47

Neaktivní uživatel

Člen

Neaktivní uživatel:10.11.2017 17:50

Tak jednoduše nepoužívej AcceptTcpClientAsync, ReadAsync a WriteAsync. Použij sync metody.

Při pokusu o čtení ti nevyskočí vyjímka. Viz. dokumentace

Nahoru Odpovědět

10.11.2017 17:50

Neaktivní uživatelský účet

Erik Šťastný

Člen

Erik Šťastný:10.11.2017 17:58

Vyskočí když je socket zavřený, právě otestováno

IOException

když nepoužiju async metody, tak nemám ponětí jak to udělat rozumně aby to dokázalo obsluhovat více klientů a do toho šlo zároveň psát do konzole.

Nahoru Odpovědět

10.11.2017 17:58

Erik Šťastný

Člen

Erik Šťastný:10.11.2017 18:04

Ha tak už vím jak to je, když klienta na tvrdo killnu, tak to vyhodí exception, ale když na klientovi zavolám TcpClient.close() tak ji to nevyhodí.

Nahoru Odpovědět

10.11.2017 18:04

Erik Šťastný

Člen

Erik Šťastný:10.11.2017 18:27

To čtení když vrací nulu si ošetřím to je tedy v pořádku, ale rád bych se vrátil celkově k tomu špatnému návrhu, zda by jsi mi ty nebo někdo mohl pomoci s tím jak to udělat do přijatelné varianty.

Jinak část kódu

private async void WaitForClientConnect()
{
    TcpClient client = await server.AcceptTcpClientAsync();
    Console.WriteLine("The async connection was created for: " + ((IPEndPoint)client.Client.RemoteEndPoint).Address.ToString());

    OnClientConnect(client);
}

čeká dokud se někdo nepřipojí :-O

Editováno 10.11.2017 18:28

Nahoru Odpovědět

10.11.2017 18:27

Neaktivní uživatel

Člen

Neaktivní uživatel:10.11.2017 20:07

Pokud nanapíšeše await před volání WaitForClientConnect(), tak opravdu nečeká. Měl bys to nastudovat.

Nahoru Odpovědět

10.11.2017 20:07

Neaktivní uživatelský účet

Erik Šťastný

Člen

Erik Šťastný:10.11.2017 20:38

A proč se tedy:

The async connection was created for...

vypíše až po tom co se někdo připojí?

Editováno 10.11.2017 20:38

Nahoru Odpovědět

10.11.2017 20:38

Neaktivní uživatel

Člen

Neaktivní uživatel:10.11.2017 20:44

Zkus přidat výpis do těchto metod:

public void ServerStart()
    {
        server.Start();
        WaitForClientConnect();
        Console.WriteLine(".....ServerStart");
    }



    private void OnClientConnect(TcpClient client)
    {
        ClientLowAPI clientReq = new ClientLowAPI(client);

        WaitForClientConnect();
        Console.WriteLine(".....OnClientConnect");
    }

Uvidíš, že ten výpis bude dřív, než se někdo stihne připojit, tedy bude to před "The async connection created for....".

Vedlejší účinek tvého návrhu je to, že to vypadá, že to funguje

Editováno 10.11.2017 20:45

Nahoru Odpovědět

10.11.2017 20:44

Neaktivní uživatelský účet

Erik Šťastný

Člen

Erik Šťastný:13.11.2017 8:07

Omlouvám se o víkendu "nepracuju" a nejsem většinou ani na PC

No tak situace, kterou popisuješ bohužel nenastala :-O viz. screenshot.

Každopádně pokud budeš mít čas a náladu budu velmi rád, když mi to pomůžeš dostat do rozumného stavu a pochopit ten async způsob, přepsat do to sync řešení jen protože to neumím bych nerad

Nahoru Odpovědět

13.11.2017 8:07

Roman

Člen

Roman:13.11.2017 19:36

Mozna bude lepsi si vykopirovat kod z MS a poradne si odkrokovat a pochopit funkci async volani

Nahoru Odpovědět

13.11.2017 19:36

Erik Šťastný

Člen

Erik Šťastný:14.11.2017 8:30

No tak pokud vím, tak na awaitu se kód uspí a program pokračuje před async metodou a až se vrátí odpověď z awaitnuté metody tak se vyvolá na tom místě ne?

Kdybych to plně chápal z nějakého examplu, tak tu nezakládám thready na fórku Nejsem ten co se ptá ještě než napíše řádek kódu nebo googluje

Nahoru Odpovědět

14.11.2017 8:30

Neaktivní uživatel

Tvůrce

Neaktivní uživatel:14.11.2017 9:05

Async/await nemá s uspáváním nic společného. Ať už je to c# nebo JavaScript.
Představ si to tak, že když dáš před volání async funkce await, tak se počká na její vyhodnocení a až pak se pokračuje sekvenčně dolů.
Ještě lepší představa by byla, kdybys náhodou znal a chápal koncept promise. Async await je v podstatě sugar pro promisy.
Každopádně představovat si to jako uspání je špatně, takhle to nefunguje.

Nahoru Odpovědět

14.11.2017 9:05

Neaktivní uživatelský účet

Erik Šťastný

Člen

Erik Šťastný:14.11.2017 9:12

Promiň špatná terminologie, uspí jsem myslel jako počká, nicméně to hlavní je ,ale to ,že pokračuje kód před tou async metodou, ne?

Nahoru Odpovědět

14.11.2017 9:12

Marian Benčat

Člen

Marian Benčat:14.11.2017 9:27

"Představ si to tak, že když dáš před volání async funkce await, tak se počká na její vyhodnocení a až pak se pokračuje sekvenčně dolů." - todle neni tak uplne 100% pravda. Na nic se nečeká. Právě, že kdyby se čekalo, tak je to k ničemu (a to hlavni vlakno ti vytuhne). Samozřejmě záleží hodně na configure await. (U STA i s await muze dispatch UI thread čekat, pokud je to špatně nastavené).Už z toho důvodu, to není stejné jako promise,..

Nahoru Odpovědět

14.11.2017 9:27

Totalitní admini..

Neaktivní uživatel

Tvůrce

Neaktivní uživatel:14.11.2017 10:06

Jen jsem se snažil přiblížit z nějaký běžný stránky věci. Vím, že to není takhle, ale vždycky jsem to tak viděl lidí vysvětlovat. Musím být přesnější, já vím.

Editováno 14.11.2017 10:07

Nahoru Odpovědět

14.11.2017 10:06

Neaktivní uživatelský účet

Neaktivní uživatel

Tvůrce

Neaktivní uživatel:14.11.2017 10:08

Tomhle úplně nerozumím, jak může pokračovat něco před? To před, už se provedlo ne?

Nahoru Odpovědět

14.11.2017 10:08

Neaktivní uživatelský účet

Erik Šťastný

Člen

Erik Šťastný:14.11.2017 10:14

pseudo:

class Program
{
    static void Main(string[] args)
    {
        Console.WriteLine("1");
        AsyncMethod();
        Console.WriteLine("2");

    }

    public static async void AsyncMethod()
    {
        await MyMethod();
    }
}

Výstup bude:
1
2

a po returnu z awaitnuté metody:

2

Takto jsem to myslel.

Editováno 14.11.2017 10:15

Nahoru Odpovědět

14.11.2017 10:14

Neaktivní uživatel

Tvůrce

Neaktivní uživatel:14.11.2017 10:21

Tohle ale je úplně jinej případ.
Já mluvím o tomhle pseudokódu:

class program
  main()
    asyncFoo()

async asyncFoo()
  print "první"
  await asyncBar()
  print "poslední"

async asyncBar()
  print "druhý"

Protože tvůj kód sice awaituje, ale jako příklad použití je to zvláštní. Možná to tak potřebuješ, to nezpochybňuju.

A z tvého kódu nevidím, jestli se vypíše ještě jedna 2ka na konci.

Editováno 14.11.2017 10:22

Nahoru Odpovědět

14.11.2017 10:21

Neaktivní uživatelský účet

Erik Šťastný

Člen

Erik Šťastný:14.11.2017 10:31

Už se v tom ztrácím, uvedomu trošku v obraz co vlastně od svého kódu očekávám.

Pustím program.

Vytvoří se server, který poslouchá pro nové klienty asynchroně, protože potřebuji aby se tok programu dostal dále k ovládací konzoli serveru.

při vytvoření z vyvolaného returnu z await nové spojení, jakož to smyčku pro nového klienta, kde se už komunikuje tam a zpátky stále dokola.

Zkusím uvést obrázek, který snad ušetří mnoho otázek a nedorozumnění

Editováno 14.11.2017 10:32

Nahoru Odpovědět

14.11.2017 10:31

Erik Šťastný

Člen

Erik Šťastný:14.11.2017 10:33

Očividně nejde v úpravě příspěvku změnit obrázek, přidávám upravený (jedna šipka na víc)

Nahoru Odpovědět

14.11.2017 10:33

Erik Šťastný

Člen

Erik Šťastný:14.11.2017 12:21

No zkusil jsem to změnit a vytvořit něco takového? Je to lepší směr nebo stále úplně špatně?
Hlavně jsem se chtěl zbavit té rekurze jak jsi říkal.

class ClientLowAPI
    {
        private TcpClient client;
        private NetworkStream stream;
        private string ip;

        public ClientLowAPI(TcpClient clientConnected)
        {
            client = clientConnected;
            stream = client.GetStream();
            ip = string.Copy(((IPEndPoint)client.Client.RemoteEndPoint).Address.ToString());

            Listening();
        }

        private async void Listening()
        {
            while (true)
            {
                try
                {
                    int length = await GetHeader();
                    byte[] message = await GetData(length);
                    byte[] asnwerMessage = ExecuteMessage(message);
                    SendMessage(asnwerMessage);
                }
                catch
                {
                    stream.Close();
                    client.Close();
                    Console.WriteLine("The async connection was closed for: " + ip + " (Username: " + HighAPI.GetUsername() + ")");
                    return;
                }

            }
        }

        private async Task<int> GetHeader()
        {
            byte[] buffer = new byte[4];

            int bytesRead = 0;
            while (bytesRead < 4)
            {
                bytesRead += await stream.ReadAsync(buffer, bytesRead, buffer.Length - bytesRead);
                if (bytesRead == 0)
                {
                    throw new System.IO.IOException();
                }
            }

            return FourBytesToInt(buffer);
        }

        private async Task<byte[]> GetData(int length)
        {
            byte[] buffer = new byte[length];

            int bytesRead = 0;
            while (bytesRead < length)
            {
                bytesRead += await stream.ReadAsync(buffer, bytesRead, buffer.Length - bytesRead);
                if (bytesRead == 0)
                {
                    throw new System.IO.IOException();
                }
            }

            return buffer;
        }

        private byte[] ExecuteMessage(byte[] binaryData)
        {
            // Do something with message
            return answer
        }

        public async void SendMessage(byte[] message)
        {
            byte[] buffer = new byte[message.Length + 4];
            byte[] length = IntToFourBytes(message.Length);

            length.CopyTo(buffer, 0);
            message.CopyTo(buffer, 4);

            await stream.WriteAsync(buffer, 0, buffer.Length);
        }

        private int FourBytesToInt(byte[] array)
        {
            int res = 0;
            res += array[0] * 256 * 256 * 256;
            res += array[1] * 256 * 256;
            res += array[2] * 256;
            res += array[3];
            return res;
        }

        private byte[] IntToFourBytes(int intValue)
        {
            byte[] array = new byte[4];
            array[0] = (byte)(intValue >> 24);
            array[1] = (byte)(intValue >> 16);
            array[2] = (byte)(intValue >> 8);
            array[3] = (byte)intValue;
            return array;
        }
    }

Editováno 14.11.2017 12:22

Nahoru Odpovědět

14.11.2017 12:21

Neaktivní uživatel

Člen

Neaktivní uživatel:14.11.2017 20:24

Radši založ nové vlákno s konkrétní otázkou. Problém s využitím CPU se zdá být vyřešený.

Mimochodem to while (true) v constructoru ti dává smysl? Takto nikdy nedojde k vytvoření...

Nahoru Odpovědět

14.11.2017 20:24

Neaktivní uživatelský účet

Marian Benčat

Člen

Marian Benčat:14.11.2017 23:00

Ty mas novou profilovku? Ty jo, sluší.

Nahoru Odpovědět

14.11.2017 23:00

Totalitní admini..

Neaktivní uživatel

Tvůrce

Neaktivní uživatel:15.11.2017 10:31

Díky :-D :-D skoro takhle vypadám i real life.

Nahoru Odpovědět

15.11.2017 10:31

Neaktivní uživatelský účet

Marian Benčat

Člen

Marian Benčat:15.11.2017 16:17

Jen míň barevnej, že? A to nejsem rasista

Nahoru Odpovědět

15.11.2017 16:17

Totalitní admini..

Neaktivní uživatel

Tvůrce

Neaktivní uživatel:15.11.2017 17:54

Ja jsem takhle bledej. :-D Hlavne ma jiny vlasy.

Nahoru Odpovědět

15.11.2017 17:54

Neaktivní uživatelský účet