Kleines Chatprogramm mit Async

Code-Stücke können hier veröffentlicht werden.
Antworten
Üpsilon
User
Beiträge: 222
Registriert: Samstag 15. September 2012, 19:23

Hallo, ich habe ein kleines TCP-Chatprogramm geschrieben unter Verwendung der Netzwerkfunktionalität des asyncio-Moduls. Vor einigen Jahren, als die Computer noch aus Holz waren und es das async-Schlüsselwort noch nicht gab, hatte ich schon mal ein Programm geschrieben, was im Wesentlichen das gleiche macht, aber direkt Sockets aus dem sockets-Modul und Threading benutzt hat. Aber Threading scheint ja aus der Mode gekommen zu sein und Async ist wohl die neue Zukunftstechnologie, deswegen wollte ich das mal ausprobieren und bitte um Rückmeldung, ob das mit dem Async so im Sinne des Erfinders ist (und ob das Programm ansonsten gut ist).

Erst einmal der Client, der ist eigentlich nichts besonderes und prinzipiell identisch mit dem was ich schon vor Jahren geschrieben hatte: Man muss Strg+C drücken, um etwas senden zu können. (Wenn ihr eine bessere Idee habt, wie man das lösen kann, ohne GUIs zu benutzen oder es sonstwie irre kompliziert zu machen, dann sprecht!)

Code: Alles auswählen

import socket

s = socket.socket()
print("--- halbwegs universeller Client ---")
ip = input("Server-IP eingeben: ")
port = int(input("Port: "))
nachricht = input("Anfangsnachricht (z.B. Nutzername): ") +"\n"
s.connect((ip,port))
s.send(nachricht.encode("utf-8"))
print("Mutmaßlich verbunden\n")

while True:
    try:
        msg = s.recv(1024)
    except KeyboardInterrupt:
        s.send((input("\t> ")+"\n").encode("utf-8"))
    else:
        print(msg.decode("utf-8"))
Jetzt der Server, der ist neu:

Code: Alles auswählen

#!/usr/bin/env python3
import asyncio
import datetime

verbindungsdict = dict()
verbindungslock = asyncio.Lock()

async def verbinden(rein, raus):
    name = (await rein.readline()).decode("utf-8").strip()
    async with verbindungslock:
        if name not in verbindungsdict.keys():
            verbindungsdict[name] = (rein, raus, raus.get_extra_info("peername"))
        else:
            return
    await sende_an_alle("Verbunden mit: "+ name + str(raus.get_extra_info("peername")))
    await asyncio.create_task(lesen(name, rein))

async def sende_an_alle(text):
    text = datetime.datetime.now().strftime("%H:%M:%S") + " " + text
    async with verbindungslock:
        kopie = dict(verbindungsdict)
    for v in kopie.values():
        v[1].write(text.encode("utf-8"))
    for v in kopie.values():
        try:
            await v[1].drain()
        except ConnectionResetError:
            print("Verbindung zurückgesetzt!")
    print(text)

async def lesen(name, rein):
    while True:
        try:
            text = await rein.readline()
        except BrokenPipeError: # Verbindung verloren
            async with verbindungslock:
                del verbindungsdict[name]
            await sende_an_alle("Verbindung verloren zu: "+ name)
            return
        text = name + ":\t" + text.decode("utf-8").strip()
        await sende_an_alle(text)

async def main():
    server = await asyncio.start_server(verbinden, "localhost", 25565) # das localhost muss ggf. geändert werden
    addr = server.sockets[0].getsockname()
    print("Servieren auf", addr)
    async with server:
        await server.serve_forever()

asyncio.run(main())
Meine Fragen sind:
- Ist das vernünftig so, wie ich das mit dem Async mache?
- Ist das mit dem Lock vernünftig?
- Wie kann man die globalen Variablen eliminieren? Gibt es noch andere Möglichkeiten als eine Klasse zu schreiben?
- Hätte es eine Möglichkeit gegeben, direkt die Sockets aus dem sockets-Modul auf "asynchrone" Art zu benutzen, anstatt dieser Streams aus dem asyncio-Modul? Ich hatte das nämlich zunächst probiert, aber das Ergebnis war, dass ich mehr oder weniger mein altes gethreadetes Programm abgeschrieben habe aber mit "Tasks" statt Threads (und das fand ich dann sinnlos und habe es gelassen). Denn viel anders ging es nach meinem Dafürhalten nicht, da die ´recv´-Methode der Sockets sich eben die Zeit nimmt bis sie etwas hat (oder eine Exception wirft, je nachdem wie man das mit dem Timeout macht), und sich nicht "awaiten" lässt.

Jede Rückmeldung zu den Programmen ist willkommen.
Liebe Grüße
PS: Die angebotene Summe ist beachtlich.
Sirius3
User
Beiträge: 17710
Registriert: Sonntag 21. Oktober 2012, 17:20

Erstmal zum Client: Einbuchstabige Variablennamen oder Abkürzungen sind generell schlecht.
s -> chat_socket, msg -> message
Der Socketcode ist wie 99.9% aller Beipiele, die man im Internet so findet, kaputt.
Du scheinst ein zeilenbasiertes Protokoll zu benutzen.
`recv` liest 1 bis 1024 Bytes. Dein Code muß also damit zurecht kommen, dass auch halbe UTF8-Squenzen gelesen werden, was bei Dir zu einem UnicodeDecodeError führt.
Dass da zerstückelte Nachrichten am Bildschirm erscheinen, ist auch nicht gerade schön.
Beim Senden ist es das selbe, da werden 1 bis alle Bytes gesendet. Da muß man also den Rückgabewert prüfen, oder sendall benutzen.
Für eine Zeilenbasiertes Protokoll ist es aber besser, socket.makefile zu benutzen.

Code: Alles auswählen

import socket

def main():
    print("--- halbwegs universeller Client ---")
    ip = input("Server-IP eingeben: ")
    port = int(input("Port: "))
    nachricht = input("Anfangsnachricht (z.B. Nutzername): ") +"\n"
    chat_socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
    chat_socket.connect((ip, port))
    file = chat_socket.makefile(mode="rw", encoding="utf8")
    file.write(nachricht)
    while True:
        try:
            message = file.readline()
        except KeyboardInterrupt:
            message = input("\t> ")
            file.write(message + "\n")
        else:
            print(message)

if __name__ == '__main__':
    main()

Beim Server gilt natürlich das selbe. Keine einbuchstabigen, oder nichtssagenden Variablennamen. Typen haben dort auch nichts verloren. verbindungsdict -> verbindungen.
Zudem sollte es keine globalen Variablen geben. Strings stückelt man nicht mit + zusammen, sondern benutzt Stringformatierung.
Bei async hat man die totale Kontrolle, wann man zwischen den Programmabläufen springt, da sind Locks sehr selten nötig, bei Dir also gar nicht.
Üpsilon
User
Beiträge: 222
Registriert: Samstag 15. September 2012, 19:23

Der Hinweis mit dem Socket, dem Stückeln und dem makefile ist interessant. Darüber hatte ich nie nachgedacht, weil die Nachrichten immer kurz genug waren, dass es funktioniert hat. Darüber was für ein "Protokoll" ich verwende, habe ich mir hier keine Gedanken gemacht. Da scheint das mit dem makefile und readline wie eine gute Idee. Funktioniert aber leider nicht: mit write gesendete Nachrichten kommen erst dann beim Server an, wenn ich den Client abschieße. Die Doku zu socket.makefile war dazu keine Hilfe, ich werde versuchen das zu ergründen und eventuell noch mal hier fragen.
Bei async hat man die totale Kontrolle, wann man zwischen den Programmabläufen springt, da sind Locks sehr selten nötig, bei Dir also gar nicht.
Interessant. Heißt das, ich kann mich darauf verlassen, dass nur bei Awaits gesprungen wird? Wenn ja, ist mir das aber immer noch etwas suspekt. Ich habe jetzt den Lock und die Kopie entfernt. Stellt man sich folgendes Szenario vor: Die Koroutine sende_an_alle läuft und kommt zu der Stelle, wo das drain awaitet wird. Darauf springt das Programm zur Koroutine lesen. Nehmen wir an, dort wird eine Verbindung gelöscht. Dann kommt das Programm wieder zu einem await, und dann springt das Programm wieder zurück in das sende_an_alle, wo zwischenzeitlich ein drain fertig geworden ist. Jetzt hat sich das Dict verbindungen ( :D ) geändert, während darüber iteriert wird. Da ich nicht so genau weiß, wie for-Schleifen in Python umgesetzt sind, weiß ich auch nicht, ob das ein Problem ist. Aber nehmen wir spaßeshalber an, ich hätte anstatt einer ´for v in verbindungen.values()´-Schleife eine ´for j in verbindungen.keys()´-Schleife, und würde mit verbindungen[j] in jedem Durchgang zugreifen. Dann wäre einer der Einträge aus dem Dict verschwunden und es gäbe einen KeyError. Dann hätte ich zwar nicht unbedingt den Lock gebraucht, aber eine Kopie von verbindungen.
Beim Server gilt natürlich das selbe. Keine einbuchstabigen, oder nichtssagenden Variablennamen.
Meinst du damit noch andere Variablennamen außer v?
Typen haben dort auch nichts verloren. verbindungsdict -> verbindungen.
Mal angenommen der Lock wäre nötig gewesen, wie hättest du ihn genannt? verbindungslock ja wohl nicht, denn Lock ist ja der Datentyp.
Zudem sollte es keine globalen Variablen geben.
Wegen *einer* globalen Variable eine Klasse zu schreiben erscheint mir überblasen und hätte zudem den Preis, dass ich ständig self schreiben müsste.
Strings stückelt man nicht mit + zusammen, sondern benutzt Stringformatierung.
Welche von den Stringformatierungen? % und format verschlechtern die Lesbarkeit, denn dann hat man einen Haufen % und {} in den Strings stehen und muss hinter dem String lesen, was ggf. ganz vorne in den String reinkommt. Ich bin jetzt auf f""-Strings umgestiegen, aber die sind etwas speziell was die Vielfalt der Anführungszeichen betrifft (f"Verbunden mit: {name}, {str(raus.get_extra_info('peername'))}") und werden in meinem Editor nicht richtig ge-syntax-highlightet.
PS: Die angebotene Summe ist beachtlich.
Sirius3
User
Beiträge: 17710
Registriert: Sonntag 21. Oktober 2012, 17:20

Bei makefile brauchst Du nach write noch ein flush.
Bei + verschlechtern die + und die Anführungszeichen die Lesbarkeit.
Und richtig erkannt, wenn sie das Wörterbuch ändern könnte, darf man nicht darüber iterieren und braucht eine Kopie. Aber wenn sich eine Verbindung zwischenzeitlich schließt, hat man genauso ein Problem.
Statt ein Wörterbuch mit Tuplen wäre auch eine Klasse besser. Wenn man wirklich Asynchronen Code schreiben will, dann auch beim Schreiben an die Klienten und nicht seriell jedem Klient nacheinander. Dann hat man auch nicht das Problem, dass sie währenddessen sich ein Klient schließt, bzw. das muß der in seiner Senden-Methode abfangen.
Üpsilon
User
Beiträge: 222
Registriert: Samstag 15. September 2012, 19:23

danke für die Antwort.
Ich hab den Server jetzt so: immer noch ohne Klassen und mit einbuchstabigen Namen, aber mit einer abgetrennten Koroutine zum Senden, und sende_an_alle startet dann Tasks. [s]Braucht man das Try-Except in ´senden´ eigentlich? Wenn der eine Task abstürzt, ist das ja nicht weiter schlimm, der Rest vom Programm läuft ja weiter.[/s]Nach Tests: braucht man doch
Ich hoffe, das ist jetzt mehr "wirklich asynchron".

Code: Alles auswählen

#!/usr/bin/env python3
import asyncio
import datetime

verbindungen = dict() # Dictionary {Nutzername: (Reader, Writer, (IP, Port)), ...}

async def verbinden(rein, raus):
    name = (await rein.readline()).decode("utf-8").strip()
    if name not in verbindungen.keys():
        verbindungen[name] = (rein, raus, raus.get_extra_info("peername"))
    else:
        print("Anfrage abgelehnt")
        return
    await sende_an_alle(f"Verbunden mit: {name}, {str(raus.get_extra_info('peername'))}\n")
    await asyncio.create_task(lesen(name, rein))

async def senden(raus, nachricht):
    raus.write(nachricht.encode("utf-8"))
    try:
        await raus.drain()
    except ConnectionResetError:
        print("Verbindung zurückgesetzt")

async def sende_an_alle(nachricht):
    nachricht = f"{datetime.datetime.now().strftime('%H:%M:%S')} {nachricht}"
    sendetasks = []
    for v in verbindungen.values():
        sendetasks.append(asyncio.create_task(senden(v[1], nachricht)))
    for t in sendetasks:
        await t
    print(nachricht)

async def lesen(name, rein):
    while True:
        try:
            nachricht = await rein.readline()
        except BrokenPipeError: # Verbindung verloren
            del verbindungen[name]
            await sende_an_alle(f"Verbindung verloren zu {name}")
            return
        nachricht = f"{name}:\t {nachricht.decode('utf-8')}"
        await sende_an_alle(nachricht)

async def main():
    server = await asyncio.start_server(verbinden, "localhost", 25565) # ggf. ändern
    print("Servieren auf", server.sockets[0].getsockname())
    async with server:
        await server.serve_forever()

if __name__ == "__main__":
    asyncio.run(main())
PS: Die angebotene Summe ist beachtlich.
Sirius3
User
Beiträge: 17710
Registriert: Sonntag 21. Oktober 2012, 17:20

Du hast immer noch globale Variablen, und viele Threads, die gar nicht gebraucht werden, weil Du sofort auf ihr Ende wartest.

Code: Alles auswählen

#!/usr/bin/env python3
import asyncio
import datetime

class Client:
    def __init__(self, rein, raus, peername):
        self.rein = rein
        self.raus = raus
        self.peername = peername

    async def senden(self, nachricht):
        try:
            self.raus.write(nachricht.encode("utf-8"))
            await self.raus.drain()
        except ConnectionResetError:
            print("Verbindung zurückgesetzt")

    async def lesen(self, server):
        while True:
            try:
                nachricht = await rein.readline().decode('utf-8')
            except BrokenPipeError: # Verbindung verloren
                break
            nachricht = f"{self.name}:\t {nachricht}"
            await server.sende_an_alle(nachricht)
        await server.sende_an_alle(f"Verbindung verloren zu {self.name}")


class Server:
    def __init__(self):
        self.verbindungen = {}

    async def run(self):
        server = await asyncio.start_server(self.verbinden, "localhost", 25565) # ggf. ändern
        print("Servieren auf", server.sockets[0].getsockname())
        async with server:
            await server.serve_forever()

    async def verbinden(self, rein, raus):
        peername = raus.get_extra_info("peername")
        name = (await rein.readline()).decode("utf-8").strip()
        if name in self.verbindungen:
            print("Anfrage abgelehnt")
            return
        client = Client(rein, raus, peername)
        self.verbindungen[name] = client
        await sende_an_alle(f"Verbunden mit: {name}, {peername}\n")
        await client.lesen(self)
        del self.verbindungen[name]

    async def sende_an_alle(self, nachricht):
        now = datetime.datetime.now()
        nachricht = f"{now:%H:%M:%S} {nachricht}"
        await asyncio.gather(*(
            client.senden(nachricht)
            for client in self.verbindungen.values()
        ))
        print(nachricht)


async def main():
    server = Server()
    await server.run()

if __name__ == "__main__":
    asyncio.run(main())
Üpsilon
User
Beiträge: 222
Registriert: Samstag 15. September 2012, 19:23

Hallo,
danke für die Antwort. Was meinst du mit "Threads, die gar nicht gebraucht werden, weil Du sofort auf ihr Ende wartest", und inwiefern macht dein Programm das anders? Meinst du damit, dass ich Tasks erstelle und die dann alle awaite, und du stattdessen gather benutzt? Das macht nach meinem Verständnis doch im Wesentlichen das Gleiche -- auch bei mir laufen die Tasks "gleichzeitig": wenn ich in die Koroutine ´sende´ ein asyncio.sleep(1) reinsetze und zwei Clients mit dem Server verbinde, dann dauert das Senden an alle circa eine Sekunde und nicht zwei, so sah es bei mir aus als ich es ausprobiert habe.
Nette Grüße
PS: Die angebotene Summe ist beachtlich.
Sirius3
User
Beiträge: 17710
Registriert: Sonntag 21. Oktober 2012, 17:20

Das in `sende_an_alle` ist einfach nur umständlich geschrieben, weil das selbe schon mit gather fertig existiert. Das meinte ich nicht.

Was macht denn ein

Code: Alles auswählen

await asyncio.create_task(lesen(name, rein))
Es erzeugt einen Task und wartet gleich darauf, dass der auch wieder beendet wird. Das ist also nichts anderes als:

Code: Alles auswählen

await lesen(name, rein)
Üpsilon
User
Beiträge: 222
Registriert: Samstag 15. September 2012, 19:23

Ah, ach so. Danke für die Antwort. Ist wie gesagt mein erstes Mal mit Async.
PS: Die angebotene Summe ist beachtlich.
Antworten