Timer / TimerTask Frage

[skywalker]

hallo,

wenn ich einen TimerTask mit einem sehr sehr kleinen intial delay aufruf - z.B. 1 millisekunde kommt es (nicht regelmäßig) vor das im Task der if-Zweig nicht ausgeführt wird.

public class SimpleBroadcastServer extends Node  {

  ArrayList<Connector> connectorList = new ArrayList<Connector>();
  Clock clock = new Clock();
  Message msg = new Message();
  Timer timer;  
  /**
   * @param seconds is the period time in seconds between successive task executions
   */
  public SimpleBroadcastServer(int seconds) {
    timer = new Timer();
    timer.scheduleAtFixedRate( new BroadcastTask(),
        1,          // initial delay in milliseconds
        seconds * 1000); // subsequent rate in seconds 
    //(converts milliseconds in seconds)     
  }
  public void connect(Connector connector) {
    connectorList.add(connector);
  }
  /**
   *  broadcastTime is used by class BroadcastTask
   *  creates a new message and new clock
   *  sets message time on current time
   *  sends generated message to the connectors in the connectorList
   */
  public void broadcastTime() { 
    for(Connector connector: connectorList) {
      msg.setTime(clock.getCurrentTime());
      System.out.println("server sends broadcast\t\t\t\t\t\t "+msg.getMessageContent());//test output
      connector.send(this, msg);          
     
    }
  }
  /**
   * send respond to the client request
   */
  public void send(Connector connectorFrom, Message msg){
    Message respondMsg = new Message();
    Clock clock = new Clock();
    respondMsg.setTime(clock.getCurrentTime());
    connectorFrom.send(this, respondMsg);
    System.out.println("server got client request and responds with  \t\t\t " 
                       + respondMsg.getMessageContent());//test output
  }
  /**
   * BroadcastTask sends broadcastTime() at regular intervals  
   */
  class BroadcastTask extends TimerTask {

    int numberMessages = 1;//defines how many messages will be sent    

    public void run() {      
      if (numberMessages > 0) {
        broadcastTime();              //DIESER TEIL WIRD "VERSCHLUCKT"
        numberMessages--;
      } else {  
        System.out.println("messages transmitted");
        timer.cancel();             
      }
    }
  }  
}

hat wer eine idee warum bzw. gibts dafür eine erklärung?

greetz, alex.

 

[foobar]

Du hast bestimmt eine Racecondition. Deklarier die run-Methode mal als synchronized.

 

[skywalker]

mit synchronized wird der if-zweig nahezu immer "geschluckt" - also nicht ausgeführt.

manchmal bekomm ich auch folgende fehlermeldung:

Exception in thread "Timer-0" java.util.ConcurrentModificationException
	at java.util.AbstractList$Itr.checkForComodification(Unknown Source)
	at java.util.AbstractList$Itr.next(Unknown Source)
	at org.trade.tsframework.core.SimpleBroadcastServer.broadcastTime(SimpleBroadcastServer.java:38)
	at org.trade.tsframework.core.SimpleBroadcastServer$BroadcastTask.run(SimpleBroadcastServer.java:65)
	at java.util.TimerThread.mainLoop(Unknown Source)
	at java.util.TimerThread.run(Unknown Source)

 

[didjitalist]

dein task wird durch den aufruf von scheduleAtFixedRate jede sekunde aufgerufen. wenn broadcastTime() jetzt länger als eine sekunde braucht, wird dein timer mehrmals aufgerufen. der nicht synchronisierte zugriff auf das feld numberMessages kann dann dafür sorgen, dass in dem feld nicht steht, was du erwartest. ein int kann zwar iirc atomar auf einen neuen wert gesetzt werden, die subtraktion ist aber keine atomare aktion.

dazu kommt das problem, dass die methode connect die liste connectorList modifziert, die zeitgleich in der methode broadcastTime durchiteriert wird (ConcurrentModificationException).

weiterhin wird in der methode broadcastTime das feld msg modifziert, ohne synchronisiert zu werden. das kann auch zu ungewollten zuständen führen.

kurz gesagt, dein nebenläufiges system ist nicht thread safe und verhält sich daher fehlerhaft.

 

[skywalker]

ok - das mit dem letzten satz hab ich mir auch so gedacht.

aus den vorigen sätzten lern ich einiges - nur wie bieg ich das ganze mit timer / timer task so hin das es thread safe ist?

 

[didjitalist]

public class SimpleBroadcastServer extends Node  { 

	  private ArrayList<Connector> connectorList = new ArrayList<Connector>(); 
	  private Clock clock = new Clock(); 
	  private Message msg = new Message(); 
	  private Timer timer;  
	  /** 
	   * @param seconds is the period time in seconds between successive task executions 
	   */ 
	  public SimpleBroadcastServer(int seconds) { 
	    timer = new Timer(); 
	    timer.scheduleAtFixedRate( new BroadcastTask(), 
	        1,          // initial delay in milliseconds 
	        seconds * 1000); // subsequent rate in seconds 
	    //(converts milliseconds in seconds)      
	  } 
	  public void connect(Connector connector) { 
		  synchronized( connectorList )
		  {
			  connectorList.add(connector);
		  }
	  } 
	  /** 
	   *  broadcastTime is used by class BroadcastTask 
	   *  creates a new message and new clock 
	   *  sets message time on current time 
	   *  sends generated message to the connectors in the connectorList 
	   */ 
	  public void broadcastTime() {
		  synchronized( connectorList )
		  {
		    for(Connector connector: connectorList) { 
		      msg.setTime(clock.getCurrentTime()); 
		      System.out.println("server sends broadcast\t\t\t\t\t\t "+msg.getMessageContent());//test output 
		      connector.send(this, msg);          
		    } 
		  }
	  } 
	  /** 
	   * send respond to the client request 
	   */ 
	  public void send(Connector connectorFrom, Message msg){ 
	    Message respondMsg = new Message(); 
	    Clock clock = new Clock(); 
	    respondMsg.setTime(clock.getCurrentTime()); 
	    connectorFrom.send(this, respondMsg); 
	    System.out.println("server got client request and responds with  \t\t\t " 
	                       + respondMsg.getMessageContent());//test output 
	  } 
	  /** 
	   * BroadcastTask sends broadcastTime() at regular intervals  
	   */ 
	  class BroadcastTask extends TimerTask { 

	    AtomicInteger numberMessages = new AtomicInteger( 1 );//defines how many messages will be sent    

	    public void run() {      
	      if (numberMessages.get() > 0) { 
	        broadcastTime();              //DIESER TEIL WIRD "VERSCHLUCKT" 
	        numberMessages.decrementAndGet(); 
	      } else {  
	        System.out.println("messages transmitted"); 
	        timer.cancel();              
	      } 
	    } 
	  }  
	}

müsste so eigentlich hinhauen. connectorList ist das objekt, auf das synchronisiert wird. die beteiligten felder sind private, damit sie nicht von aussen modifziert werden können. die anzahl der messages wird in einem AtomicInteger vorgehalten, der dafür sorgt, dass wertmanipulationen atomar bleiben.

 

[didjitalist]

wenn die methode Connector#send nicht blockierend ist, solltest du weiterhin nicht mit dem feld Message arbeiten, sondern immer ein frisches objekt erzeugen.

 

[skywalker]

cool, auf den ersten blick funktioniert das so.

wenn die methode Connector#send nicht blockierend ist, solltest du weiterhin nicht mit dem feld Message arbeiten, sondern immer ein frisches objekt erzeugen.

noch eine (vielleicht blöde) frage - wo würd ich das frische msg objekt erzeugen?

 

[skywalker]

:cry: leider zu früh gefreut - der fehler tritt noch immer auf...