2016-07-09 18 views
1

Code unten funktioniert und sendet Nachricht zur geplanten Zeit, aber ich denke, es ist keine gute Lösung, neue Sockets jedes Mal zu öffnen, wenn Timer geplante Aufgabe ausführt. Was ich möchte, ist, den Socket in der run-Methode nur einmal zu öffnen und in der SendMessage-Klasse darauf zuzugreifen, wenn eine neue Instanz der Klasse im Timer erstellt wird. Auf diese Weise funktioniert es nicht, es sendet nur eine Nachricht und hört dann auf zu senden. Auch würde ich mich für einige Kritiker über Code oder Tipps freuen, die es threadsicher machen.Socket sendet Nachricht nur einmal

public class Client implements Runnable{ 

// Client Constructor here 

@Override 
public void run(){ 
    //SENDS ONLY ONE MESSAGE 
    pitcherSocket = new Socket(InetAddress.getByName(hostname), port); 

    Timer timer = new Timer(); 

    timer.schedule(new SendMessage(), 0, 1000/mps); 
} 

private class SendMessage extends TimerTask{ 

    private int id; 

    @Override 
    public void run() { 

     try 
      { // THIS WORKS FINE, SENDS MESSAGES AT SCHEDULED TIME      
      pitcherSocket = new Socket(InetAddress.getByName(hostname), port); 

      OutputStream outToServer = pitcherSocket.getOutputStream(); 

      DataOutputStream out = new DataOutputStream(outToServer); 

      out.writeInt(id); 

      out.flush(); 

      }catch(IOException e) 
      { 
      e.printStackTrace(); 
      } 
     } 
    } 
} 

EDIT: GANZE CODE

CLIENT

import java.io.DataInputStream; 
import java.io.DataOutputStream; 
import java.io.IOException; 
import java.io.InputStream; 
import java.io.OutputStream; 
import java.net.InetAddress; 
import java.net.Socket; 
import java.net.UnknownHostException; 
import java.util.ArrayList; 
import java.util.Collections; 
import java.util.List; 
import java.util.Timer; 
import java.util.TimerTask; 

public class Pitcher implements Runnable{ 

private int port; 
private int mps; 
private int size; 
private String hostname; 
private List<Integer> messageIds = Collections.synchronizedList(new  ArrayList<Integer>()); 
private Socket pitcherSocket; 

//constatns, integer is 4 bytes, long is 8 bytes 
private static final int INT_SIZE = 4; 
private static final int LONG_SIZE = 8; 


public Pitcher(int port, int mps, int size, String hostname) { 

    this.port = port; 
    this.mps = mps; 
    this.size = size; 
    this.hostname = hostname; 
} 

@Override 
public void run(){ 

    System.out.println("Pitcher running..."); 
    System.out.println(); 

    Timer timer = new Timer(); 

    timer.schedule(new SendMessage(), 0, 1000/mps); 

    timer.schedule(new DisplayStatistics(), 0, 1000/mps); 

} 

//Nested class that sends messages 
private class SendMessage extends TimerTask{ 

    private int numberOfSentMessages = 0; 
    private int id; 

    @Override 
    public void run() { 

     try {       
      pitcherSocket = new Socket(InetAddress.getByName(hostname), port); 

      OutputStream outToServer = pitcherSocket.getOutputStream(); 

      DataOutputStream out = new DataOutputStream(outToServer); 

      //send message size 
      out.writeInt(size); 

      //message id is same as number of the sent message 
      id = numberOfSentMessages + 1; 
      out.writeInt(id); 
      messageIds.add(id); 



      //get system timestamp 
      long currentTimestamp = System.currentTimeMillis(); 
      out.writeLong(currentTimestamp); 

      //fill in the rest- 
      byte[] rest = new byte[size - 2 * INT_SIZE - LONG_SIZE];  //message size(default 300 bytes) - size(4 bytes) - message id(4 bytse) - timestamp(8 bytes) 
      out.write(rest); 

      out.flush(); 

      numberOfSentMessages++; 


      InputStream inFromServer = pitcherSocket.getInputStream(); 
      DataInputStream in = new DataInputStream(inFromServer); 

      Integer catcherMessageSize = in.readInt(); 
      Integer catcherId = in.readInt(); 
      long catcherTimestamp = in.readLong(); 

      System.out.println("Sent message:  " + size + " " + id + " " + currentTimestamp + "..."); 
      System.out.println("Received message: " + catcherMessageSize + " " + catcherId + " " + catcherTimestamp + "..."); 
      System.out.println(); 

      }catch(IOException e) 
      { 
      e.printStackTrace(); 
      } 

    } 

} 

} 

SERVER

import java.io.DataInputStream; 
import java.io.DataOutputStream; 
import java.io.IOException; 
import java.net.InetAddress; 
import java.net.ServerSocket; 
import java.net.Socket; 
import java.net.SocketTimeoutException; 

public class Catcher implements Runnable{ 

private int port; 
private String bind; 
private ServerSocket serverSocket; 

//constatns, integer is 4 bytes, long is 8 bytes 
private static final int INT_SIZE = 4; 
private static final int LONG_SIZE = 8; 

public Catcher(int port, String bind) { 

    this.port = port; 
    this.bind = bind; 
} 

@Override 
public void run() { 

    System.out.println("Catcher running..."); 
    System.out.println(); 

    try { 
     serverSocket = new ServerSocket(port, 100, InetAddress.getByName(bind)); 
    } 
    catch (IOException e1) { 
     e1.printStackTrace(); 
    } 

    while(true){ 

     try 
     {    
      Socket server = serverSocket.accept(); 

      DataInputStream in = new DataInputStream(server.getInputStream()); 

      Integer pitcherMessageSize = in.readInt(); 
      Integer pitcherId = in.readInt(); 
      long pitcherTimestamp = in.readLong(); 

      DataOutputStream out = new DataOutputStream(server.getOutputStream()); 

      //message id and size are sent back 
      out.writeInt(pitcherMessageSize); 
      out.writeInt(pitcherId); 

      //send back current time 
      long currentTimestamp = System.currentTimeMillis(); 
      out.writeLong(currentTimestamp); 

      //fill in the rest 
      byte[] rest = new byte[pitcherMessageSize - 2 * INT_SIZE - LONG_SIZE]; //message size(default 300 bytes) - size(4 bytes) - message id(4 bytes) - timestamp(8 bytes) 
      out.write(rest); 

      out.flush(); 

      System.out.println("Received message: " + pitcherMessageSize + " " + pitcherId + " " + pitcherTimestamp + "..."); 
      System.out.println("Sent message:  " + pitcherMessageSize + " " + pitcherId + " " + currentTimestamp + "..."); 
      System.out.println(); 

      //server.close(); 

     } 
     catch(SocketTimeoutException s){ 
      System.out.println("Socket timed out!"); 
      break; 
     } 
     catch(IOException e){ 
      e.printStackTrace(); 
      break; 
     } 
     } 
} 
} 
+0

sehen Sie irgendeine Ausnahme? Wenn es aufhört zu senden, endet das Programm oder hängt es gerade? – niceman

+0

gibt es keine Ausnahme, der Server erhält die erste ID und hört weiter zu. Der Client-Timer führt immer noch die SendMessage-Klasse aus und versucht zu senden, aber nicht nach der ersten Iteration wird gesendet. – asdf

+0

Von dem, was ich sehe, erstellen und ausführen Sie 'SendMessage' auf einer geplanten Basis. 'SendMessage' versucht jedes Mal, wenn es erstellt wird, die Verbindung über einen' Socket' wiederherzustellen. –

Antwort

-2

Die Java-Socket-Klasse nicht sicher ist, fädeln. Wenn mehrere Threads auf dasselbe Socket-Objekt zugreifen sollen, müssen Sie ihre Aktionen synchronisieren. Dies könnte geschehen, indem Sie alle Ihre SendMessage-Threads mit einem gemeinsamen Objekt versehen, das dann als Sperre fungiert. Sie benötigen ein Objekt für jede Socket-Operation, die Sie verwenden möchten (z. B. Lesen und Schreiben). Richten Sie dann jede Aktion, die einen Aufruf des Socket-Objekts ausführt, in separate Methoden um und synchronisieren Sie sie um das Objekt herum. Z.B. Für die Leseoperation könnte eine Methode namens read() in SendMessage verwendet werden, die Socket.read aufruft und diese Methode um das lock-Objekt zum Lesen synchronisiert.

private class SendMessage extends TimerTask{ 

    private Object readLock; 
    private Socket socket; 

    public SendMessage(Object readLock, Socket socket) { 
     this.readLock = readLock; 
     this.socket = socket; 
    } 

    public void readFromSocket() { 
     synchronized(readLock) { 
       socket.read(); 
     } 
    } 

    @Override 
    public void run() { 
     readFromSocket(); 
     // do other stuff 
    } 

} 
+0

Dies erklärt nicht die Ursache seines Problems. –

+0

Das Problem tritt nur auf, wenn er dasselbe Socket-Objekt in mehreren Threads verwendet. Daher ist es wahrscheinlich, dass der Sockel nicht threadsicher ist. Bitte erläutern Sie, warum Sie nicht denken, dass dies die Ursache des Problems ist. – Soggiorno

+0

Relevant: http://stackoverflow.com/questions/13545578/is-java-socket-multi-thread-safe – Soggiorno

0

Haben Sie darüber nachgedacht, Socket und die DataOutputStream-Membervariablen von SendMessage zu erstellen. Dies ist ein Code, der Ihnen einen ungefähren Start ermöglicht. Sie werden wahrscheinlich Ich denke, setzen wollen einige Verbesserungen wie die Überprüfung, ob die Socket geöffnet ist und in der Lage, einen neuen zu erstellen, wenn die aktuellen geschlossen ist ...

private class SendMessage extends TimerTask { 
    private int id = 10; 
    private Socket pitchSocket; 
    private DataOutputStream out; 

    public SendMessage(Socket socket) { 
     this.pitchSocket = socket; 
     try{ 
      out = new DataOutputStream(pitchSocket.getOutputStream()); 
     } catch(IOException e) { 
      e.printStackTrace(); 
     } 
    } 

    @Override 
    public void run() { 
     try { 
      out.writeInt(id); 
      out.flush(); 
     } catch(IOException e) { 
      e.printStackTrace(); 
     } 
    } 
} 
+0

Noch benimmt sich das gleiche:/ – asdf

+0

Ich denke, ich könnte falsch gelesen haben Sie sind ursprünglicher Beitrag. Erstellen Sie mehr als einen SendMessenger? – CodeJockNYC

0

Nachdem in der Lage, den gesamten Code zu sehen, dass Sie haben definitiv einige Threading-Probleme, obwohl ich denke, dass sie eher auf der Server-Seite als auf der Client-Seite sind. Ihr Server ist single threaded. Das bedeutet, dass Sie jeweils nur eine Anfrage bearbeiten können. Sie möchten einen Multithread-Server. Ich habe Ihren Code überarbeitet, um ein Beispiel für Catcher zu erstellen, das Multithread ist. Ich benutze die Thead-Klasse, um all das zu tun, was ein bisschen altmodisch sein kann. Vielleicht möchten Sie einen Blick auf java.util.concurrent werfen, sie werden wahrscheinlich aktueller sein.

package clientserver; 

import java.io.DataInputStream; 
import java.io.DataOutputStream; 
import java.io.IOException; 
import java.net.InetAddress; 
import java.net.ServerSocket; 
import java.net.Socket; 
import java.net.SocketTimeoutException; 

public class Catcher implements Runnable{ 

private int port; 
private String bind; 
private ServerSocket serverSocket; 



public Catcher(int port, String bind) { 

    this.port = port; 
    this.bind = bind; 
} 

@Override 
public void run() { 

    System.out.println("Catcher running..."); 
    System.out.println(); 

    try { 
     serverSocket = new ServerSocket(port, 100, InetAddress.getByName(bind)); 
    } 
    catch (IOException e1) { 
     e1.printStackTrace(); 
    } 

    while(true){ 
     try 
     {    
      new Thread(new CatcherHandler(serverSocket.accept())).start(); 
      Thread.sleep(1000); 

     } 
     catch(SocketTimeoutException s){ 
      System.out.println("Socket timed out!"); 
      break; 
     } 
     catch(IOException e){ 
      e.printStackTrace(); 
      break; 
     } catch (InterruptedException e) { 
      // TODO Auto-generated catch block 
      e.printStackTrace(); 
     } 
    } 
} 

public static void main(String[] argv){ 
    new Thread(new Catcher(8093, "localhost")).start();; 

} 
} 

class CatcherHandler implements Runnable{ 
    Socket server; 
    DataOutputStream out; 
    DataInputStream in; 

    private static final int INT_SIZE = 4; 
    private static final int LONG_SIZE = 8; 

    public CatcherHandler(Socket server) { 
     super(); 
     this.server = server; 
     try { 
      in = new DataInputStream(server.getInputStream()); 
      out = new DataOutputStream(server.getOutputStream()); 

     } catch (IOException e) { 
      // TODO Auto-generated catch block 
      e.printStackTrace(); 
     } 

    } 



    @Override 
    public void run() { 
     try{ 
      if(in.available() > 0){ 

       Integer pitcherMessageSize = in.readInt(); 
       Integer pitcherId = in.readInt(); 
       long pitcherTimestamp = in.readLong(); 

       //message id and size are sent back 
       out.writeInt(pitcherMessageSize); 
       out.writeInt(pitcherId); 

       //send back current time 
       long currentTimestamp = System.currentTimeMillis(); 
       out.writeLong(currentTimestamp); 

       //fill in the rest 
       byte[] rest = new byte[pitcherMessageSize - 2 * INT_SIZE - LONG_SIZE]; //message size(default 300 bytes) - size(4 bytes) - message id(4 bytes) - timestamp(8 bytes) 
       out.write(rest); 

       out.flush(); 

       System.out.println("Received message: " + pitcherMessageSize + " " + pitcherId + " " + pitcherTimestamp + "..."); 
       System.out.println("Sent message:  " + pitcherMessageSize + " " + pitcherId + " " + currentTimestamp + "..."); 
       System.out.println(); 
       Thread.sleep(1000); 

      } 
     } catch (IOException e) { 
      // TODO Auto-generated catch block 
      e.printStackTrace(); 
     } catch (InterruptedException e) { 
      // TODO Auto-generated catch block 
      e.printStackTrace(); 
     }finally{} 
     //server.close(); 

    } 
} 

Zusätzlich habe ich Ihren Client umgestaltet, um eine Steckdose benutzen zu können und sicher zu sein. Jetzt nimmt SendMessage einen DataInputStream und ein DataOutputSteam als Argumente auf.

import java.io.DataInputStream; 
import java.io.DataOutputStream; 
import java.io.IOException; 
import java.io.InputStream; 
import java.io.OutputStream; 
import java.net.InetAddress; 
import java.net.Socket; 
import java.net.UnknownHostException; 
import java.util.ArrayList; 
import java.util.Collections; 
import java.util.List; 
import java.util.Timer; 
import java.util.TimerTask; 

public class Pitcher implements Runnable{ 

private int port; 
private int mps; 
private int size; 
private String hostname; 
private List<Integer> messageIds = Collections.synchronizedList(new  ArrayList<Integer>()); 
private Socket pitcherSocket; 
private DataOutputStream out; 
private DataInputStream in; 

//constatns, integer is 4 bytes, long is 8 bytes 
private static final int INT_SIZE = 4; 
private static final int LONG_SIZE = 8; 


public Pitcher(int port, int mps, int size, String hostname) { 

    this.port = port; 
    this.mps = mps; 
    this.size = size; 
    this.hostname = hostname; 



    try { 
     this.pitcherSocket = new Socket(InetAddress.getByName(hostname), port); 
     out = new DataOutputStream(pitcherSocket.getOutputStream()); 
     in = new DataInputStream(pitcherSocket.getInputStream()); 
    } catch (IOException e) { 
     // TODO Auto-generated catch block 
     e.printStackTrace(); 
    } 


} 

public static void main(String[] argv) throws Exception{ 
    for(int i = 0; i < 10; i++){ 
     new Thread(new Pitcher(8093, 1, 200, "localhost")).start(); 
     Thread.sleep(1000); 
    } 

    Thread.sleep(10000); 
} 

@Override 
public void run(){ 

    System.out.println("Pitcher running..."); 
    System.out.println(); 

    Timer timer = new Timer(); 

    timer.schedule(new SendMessage(out, in), 0, 1000); 

    //timer.schedule(new DisplayStatistics(), 0, 1000); 

} 

//Nested class that sends messages 
private class SendMessage extends TimerTask{ 

    private int numberOfSentMessages = 0; 
    private int id; 
    private DataOutputStream out; 
    private DataInputStream in; 

    public SendMessage(DataOutputStream out, DataInputStream in){ 
     this.out = out; 
     this.in = in; 
    } 

    @Override 
    public void run() { 

     try {       
      long currentTimestamp = 0L; 
      synchronized(out){ 
       //send message size 
       out.writeInt(size); 

       //message id is same as number of the sent message 
       id = numberOfSentMessages + 1; 
       out.writeInt(id); 
       messageIds.add(id); 



       //get system timestamp 
       currentTimestamp = System.currentTimeMillis(); 
       out.writeLong(currentTimestamp); 

       //fill in the rest- 
       byte[] rest = new byte[size - 2 * INT_SIZE - LONG_SIZE];  //message size(default 300 bytes) - size(4 bytes) - message id(4 bytse) - timestamp(8 bytes) 
       out.write(rest); 

       out.flush(); 
      } 
      numberOfSentMessages++; 

      long catcherTimestamp = 0L; 
      Integer catcherMessageSize; 
      Integer catcherId; 
      synchronized(in){ 
       catcherMessageSize = in.readInt(); 
       catcherId = in.readInt(); 
       catcherTimestamp = in.readLong(); 
      } 
      System.out.println("Sent message:  " + size + " " + id + " " + currentTimestamp + "..."); 
      System.out.println("Received message: " + catcherMessageSize + " " + catcherId + " " + catcherTimestamp + "..."); 
      System.out.println(); 
      Thread.sleep(1000); 

      }catch(IOException e) 
      { 
      e.printStackTrace(); 
      } catch (InterruptedException e) { 
      // TODO Auto-generated catch block 
      e.printStackTrace(); 
     } 

    } 

} 

} 
+0

Vielen Dank! Ich werde versuchen, diesen Code morgen umzusetzen und Sie wissen zu lassen, ob alles in Ordnung ist. Kannst du mir bitte sagen, warum benutzt du thread.sleep (X)? Ich bin neu in Threads. – asdf

+0

Ja, ich benutze den Schlaf um andere Threads laufen zu lassen. Wenn Sie die Methode "sleep" aufrufen, kann der Scheduler grundsätzlich andere Threads ausführen lassen. – CodeJockNYC

+0

Ich habe ein anderes Problem. Wenn der Pitcher eine Nachricht sendet, werden die Catcher System.currentTimeMillis(); ist kleiner als Krüge. – asdf