# BufferedStream funktioniert nicht immer



## Spiderpic98 (7. Jul 2022)

Guten Tag,

ich habe ein Problem mit meinem Code. Ich versuche einen Chat zu erstellen und übergebe dem Server mithilfe von Write eine Naricht die dann der Server in der Konsole mit readline() ausgeben soll. Das komische ist nun, es funktioniert nur im Schnitt jedes 2. mal und ich verstehe nicht wieso


```
public class MessageController extends Thread {
  public static BufferedWriter out;
  public static BufferedReader in;

  public MessageController() throws IOException {

    System.out.println("MessageController gestartet");

  }

  @Override
  public void run() {
    try {
    BufferedWriter  out = new BufferedWriter(new OutputStreamWriter(Server.client.getOutputStream()));
    BufferedReader  in = new BufferedReader(new InputStreamReader(Server.client.getInputStream()));
    } catch (IOException e) {
      throw new RuntimeException(e);
    }

    while(true){
      System.out.println("Message wird gelesen");
      try{

          System.out.println("MessageController erhalten: " + in.readLine());

      }catch(IOException e){
        e.printStackTrace();
        System.out.println("fehler");
      }

    }


  }



}
```
Das ist der MessageListener der auf eine Naricht wartet. Dort wird der Output und Input vom Server erstellt.


```
try {

                            sendMessage();
                            System.out.println("Naricht wird gesendet");
                            System.out.println("ChatPane: " + messageText);
                            output.write(messageText);
                            output.newLine();
                            output.flush();
                            System.out.println("Naricht wurde gesendet");

                        } catch (IOException e) {
                            throw new RuntimeException(e);
                        }
```

Das ist der Ort an dem Die Naricht losgeschickt wird. Die Streams werden in der Methode sendMessage() erzeugt.


```
public class Server extends Thread
{

    public static ServerSocket server;

    public static Socket client;
    int port;

    public Server(int port) throws IOException
    {
       this.port = port;
        server = new ServerSocket(port);
        client = new Socket("localhost",port);

        System.out.println(port);
     //    server.setSoTimeout(10000);
    }


    @Override
    public void run() {

      try {
        ConnectionListener connectionListener;
        connectionListener = new ConnectionListener();
        connectionListener.start();
        MessageController messageController;
        messageController = new MessageController();
        messageController.start();
      } catch (IOException e) {
        throw new RuntimeException(e);
      }


      while (!server.isClosed()) {

        if (!client.isConnected()) {
          System.out.println("waiting for client");
        }

      }

    }

}
```

Und das ist der Server. Der Thread ConnectionListener macht nichts anderes als die accept() Methode abwickeln.
Ich habe jeweils einen Socket im Client und einem im Server. Ich verstehe nicht warum es mal funktioniert und mal nicht. Laut der Konsole führt er jedes mal alles bis


```
System.out.println("MessageController erhalten: " + in.readLine());
```

hierhin aus und dann wartet er auf input. Und ich vermute mal entweder es erreicht ihn gar nicht erst was oder er kann nichts damit anfangen was er bekommt.
Hat jemand vielleicht Rat?


----------



## KonradN (8. Jul 2022)

Was ist das genaue Fehlerbild? Was passiert denn genau?

Was sein kann: Du startest mehrere Threads direkt hintereinander. Da nicht der ganze Code zu sehen ist, kann man nur raten, aber es wäre z.B. möglich, dass der Server Thread den Socket noch nicht richtig erstellt hat, wenn der Client bereits seinen connection Requests abgesetzt hat. Dann schlägt der Connection Request direkt fehl, der Server öffnet dann den Thread und es kommt aber kein Client mehr, da Du ja nur einen einzigen Client startest.

Lösung in so einem Fall wäre, dass man wirklich erst den Server bis zu Ende erstellt, ehe man den Client aufruft oder das in zwei Applikationen packt. Dann rufst Du erst den Server auf und wenn der Server läuft, dann rufst Du den Client zusätzlich auf.

Generell ist der Aufbau so aber auch extrem "mangelhaft". Da so mit statischen Variablen zu arbeiten, die auch public sind, würde ich schlicht als falsch bezeichnen.

Hier sollte man Variablen private und nicht statisch haben. Wenn eine Klasse eine Instanz auf eine andere Klasse. braucht, dann sollte man ihr diese geben (z.B. im Konstruktor). So würde man dann saubere Abhängigkeiten bekommen und man hätte auch eine Kapselung so dass man einfacher Elemente ändern kann ohne dann an mehreren Stellen massiv Änderungen vornehmen zu müssen.


----------



## KonradN (8. Jul 2022)

Wie so eine Server Implementierung etwas aussehen könnte, die mit Threads arbeitet, habe ich einmal auf die Schnelle unter








						java-forum-algorithms/src/main/java/de/kneitzel/net at master · kneitzel/java-forum-algorithms
					

Algorithms written for Java-Forum.org threads. Contribute to kneitzel/java-forum-algorithms development by creating an account on GitHub.




					github.com
				



skizziert.

Ist aber noch nicht getestet und der Client fehlt noch ... Und Nachrichten, die der Server empfängt sendet er einfach an alle (anderen) verbundenen Clients weiter.


----------



## mihe7 (8. Jul 2022)

TcpServer, Zeile 62, soll vermutlich clientsCopy sein.


----------



## KonradN (8. Jul 2022)

mihe7 hat gesagt.:


> TcpServer, Zeile 62, soll vermutlich clientsCopy sein.


Ja selbstverständlich. Da habe ich beim runter tippen gepennt. Danke für dem Hinweis.

Edit: Und jetzt auch behoben (incl. zusätzlichen Hinweisen im JavaDoc).


----------



## KonradN (8. Jul 2022)

Evtl. auch hier noch kurz ein paar Hinweise zu dem Code, da es ja auch um die Vermeidung von so public static Variablen ging.
(Ich habe noch ein paar weitere Änderungen eingebaut, aber immer noch ungetestet. Sorry!)

Man baut es als Objekte auf. So habe ich einen TcpServer als Klasse aufgebaut.

- Dieser soll ja auf einem Port hören, daher wird beim Konstruktor der port übergeben und da dann auch erst einmal gespeichert.

- Dann will man ja, dass dieser TcpServer auf dem Port hören kann. Dazu habe ich die Methode start (ehemals listen) erstellt:
Da wir hier mit Threads arbeiten wollen, habe ich dazu einfach einen einfachen Thread verwendet, der dann auch in der Klasse gespeichert wird.
-> Wenn es schon einen Thread gibt und dieser noch läuft, dann werfen wir eine Exception. Man kann den TcpServer sozusagen nur einmal starten. 
-> Dann wird ein neuer Thread erzeugt, der einfach listenForNewConnections aufruft.

- listenForNewConnections
--> Es wird ein ServerSocket gestartet. Das war zuerst in einem try with resources, so dass der auf jeden Fall sicher geschlossen wird, aber man braucht diesen Socket auch für die close() Methode. Daher wurde das eine Instanzvariable.
--> So lange der ServerSocket nicht geschlossen ist wird in einer Schleife:
----> Auf eine neue Connection gewartet.
----> Für jede Connection wird eine Instanz von TcpServerClient erstellt und in einer Liste von clients verwaltet.

- TcpServer hat Ressourcen intern, die man ggf. schließen können will. Man will ja alles stoppen -> Autoclosable ist da das Interface, das einem in den Sinn kommen soll mit der close Methode. Diese Methode schließt den Socket und auch alle TcpServerClients.


Der TcpServerClient bekommt dann den Socket und auch eine Referenz auf den TcpServer. (Hier wäre ggf. ein Interface interessant. Es wird ja nur eine Schnittstelle benötigt um halt empfangene Nachrichten weiter zu geben und so. Da könnte man dann auch eine Klasse für die Daten übergeben.)
Im Konstruktor wird dann der writer erstellt und der Thread zum lesen von Nachrichten gestartet.

Der Thread erstellt einfach einen Reader und liest immer von diesem. Jede Nachricht wird dem TcpServer signalisiert. (Da kann man auch ein Interface MessageReceiver oder so bauen. Das muss ja nicht der TcpServer sein.)
Unschön ist hier noch: Da wird immer mit einer Exception raus gegangen. Da sollte man einen sauberen Exit ermöglichen. Aber erst einmal für die Kernfunktionalität egal.

Man kann eine Nachricht senden und man hat das close(), das alles schließt. 

Das wäre so der grobe Aufbau in einem ersten ganz schnellen Entwurf. Wirklich so herunter geschrieben was man daran merkt, wie massiv die Anpassungen zwischen dem ersten Post und der neuen Überarbeitung sind.


----------



## Spiderpic98 (8. Jul 2022)

KonradN hat gesagt.:


> Wie so eine Server Implementierung etwas aussehen könnte, die mit Threads arbeitet, habe ich einmal auf die Schnelle unter
> 
> 
> 
> ...


Danke für die Antworten! Das Beispiel hilft sehr. Eine Frage hätte ich noch zu dem Client. Im Konstruktor wird einmal der Socket und der Serversocket übergeben. Warum? Der Socket wäre meine Vermutung damit jeder Client auch einen Socket hat durch die Narichten fließen können, aber ich verstehe nicht ganz warum der Serversocket übergeben werden muss?


----------



## KonradN (8. Jul 2022)

Also der TcpServerClient ist nicht der Client der Anwendung sondern im TcpServer die Verwaltung eines verbundenen Clients!

Und der Socket wird übergeben, da dieser ja die Verbindung zum Client ist.
Der TcpServer wird übergeben, da dies ja der eigentliche Server ist. Mit den Nachrichten des Servers muss ja irgendwas gemacht werden.

Evtl. hilft es etwas, wenn man das etwas mehr wie Events aufzieht? Dann hätte man sowas wie ein Message das Sender (Object) und Message (String) hat. Und dann hat man ein Interface MessageConsumer, das eine Methode receive(Object) hat. Dann würde TcpServer dieses Interface implementieren und der Parameter wäre dann der MessageConsumer? (Aber halt immer noch diese TcpServer Instanz. Aber wir hätten da die Abhängigkeit schon zu einem Interface gewandelt.

Das kann man auch noch weiter treiben. Der Server muss die TcpServerClient nicht verwalten. Das gehört ja nicht zwangsläufig zu einem TcpServer, der nur die Aufgabe hat, auf neue Verbindungen zu warten. Das kann dann also auch so aussehen:

- ChatServer
--> Hat einen TcpServer der auf neue Verbindungen warten.
--> Hat Clients. Diese Klasse kann sonst was beinhalten. Wenn man IRC als Protokoll anschaut, dann könnte da halt einiges an Informationen gespeichert sein. Aber es hält auch den TcpServerClient als Verbindung.
--> Der ChatServer würde dann halt der MessageConsumer sein.

Das wird dann aber recht viel und das geht halt über eine einfache Struktur für so eine reine ServerSocket Lösung hinaus.


----------

