Synchronized-Schlüsselwort bei Inkrementierung einer statischen Variable

[Shams]

Ich habe hier 2 Threads und will jeweils eine statische Variable von 0 nach 99 inkrementieren.

import java.io.*;

public class Main implements Runnable
{
	
	public static int count = 0;
	
	public int getCount() {
		return count;
	}


	 
	public synchronized void increment(){
		
        int zahl = getCount();
		
     while(zahl<100){
    	 System.out.println(zahl);
        zahl++;
     }
        
		
	}

	@Override
	public void run() {		
		increment();
	  }
 
	
	public static void main(String[] args) 
	{

      Main m1 = new Main();
      m1.run();
      
      Main m2 = new Main();
      m2.run();
    }


       
	 
}

Nun meine Frage: Ist dies mit dem Schlüsselwort "synchronized" auch dann threadsafe, wenn die zu inkrementierende Variable statisch ist? Oder kann es passieren, dass der zweite Thread irgendwann auf die bereits durch den ersten Thread inkrementierte Variable zugreifen könnte?

 

[Admiral Helmut]

ich glaube du wirst folgendes Problem kriegen.

Du vermischt static mit objectbezogen.

Mit dem synchronized auf eine Object Methode sicherst du den "Schlüssel des Objects".
D.h. es kann nur ein Thread gleichzeitig eine synchronised Methode DIESES Objekts aufrufen. NIcht mehr nicht weniger.
Es können zum Beipiel in 2 verschiedenen Objekten gleichzeitig die synchronized Methode aufgerufen werden. Und das machst du hier.
Damit ist nicht gesichert, dass nicht 2 Threads auf ein static Feld der Klasse zugreifen.

Ich denke du solltest eher so vorgehen. Du erstellst eine Datenklasse mit einem NICHT static Counter. Diese Klasse sollte dann auch eine synchronized Increment Methode haben.

Jetzt erstellst du dir noch eine Thread Klasse die im Konstruktor das Datenobjekt bekommt und sichert. und in einer Methode die Incrementmethode der Datenklasse aufruft.

In der Main erstellst du ein Datenobjekt und 2 ThreadKlassenobjekte. Diesen gibst du dann bei new das Datenobjekt mit.
Dann bist du Threadsafe.

Bitte verbessert mich wenn ich was falsches sage.

Gruß Helmut

 

[Joose]

public synchronized void increment(){	
        int zahl = getCount();	
     while(zahl<100){
    	 System.out.println(zahl);
        zahl++;
     }	
}

Gerade dieser Teil ist auch sehr unsinnig irgendwie.
Durch das "synchronized" blockierst du für andere Aufrufer die Methode.
Der 1.Thread zählt nun gemütlich hoch, wenn dieser fertig ist dann kommt der 2.Thread und hat nichts mehr zu tun

Was du glaub ich willst: Eine Methode welche deine Schleife enthält und innerhalb der Schleife rufst du eine "synchronized" Methode auf die nichts anderes macht als deinen Zähler zu inkrementieren.

 

[Shams]

import java.io.*;

public class Main implements Runnable
{
	
	public static int count = 0;
	
	public int getCount() {
		return count;
	}


	 
	public synchronized void increment(){
		
        int zahl = getCount();
         zahl+=9;
         System.out.println(zahl);
     
        
		
	}

	@Override
	public void run() {		
		increment();
	  }
 
	
	public static void main(String[] args) 
	{

      Main m1 = new Main();
      Main m2 = new Main();
      
      do{
      m1.run();
      m2.run();
       }while(true);
    }	 
}

Vielleicht ist es so etwas sinnvoller, in der Tat, also die Frage ist, ob es passieren kann, dass nicht immer wieder die 9 ausgedruckt wird, sondern auch mal 18 dastehen kann, weil trotz synchronized eine bereits auf den Wert 9 inkrementierte Variable 9 dann zu 18 gemacht werden kann.

 

[Admiral Helmut]

Meines Wissens bringt synchronized überhaupt nichts wenn man dann mit 2 verschiedenen Objekten arbeitet

Gruß Helmut

 

[Shams]

Dann ist dies ein besseres Beispiel:

import java.io.*;

public class Main implements Runnable
{
	
	public static int count = 0;
	
	public int getCount() {
		return count;
	}

	public void setCount(int z){
		count=z;
	}
	 
	public synchronized void increment(){
		
		while(count<66){
		
        int zahl = getCount();
         zahl++;
        setCount(zahl);
         System.out.println(zahl);
     
		} 
		
	}

	@Override
	public void run() {		
		increment();
	  }
 
	
	public static void main(String[] args) 
	{
	      Main m1 = new Main();
      new Thread(m1).start();
      new Thread(m1).start();

    }	 
}

 

[Admiral Helmut]

hast du es mal laufen lassen? wird die Reihenfolge vom anderen Thread unterbrochen?

zeig uns mal bitte die Ausgabe.

Die while schleife macht wie Joose schon geschrieben hat kaum einen Sinn.

Ob das mit deinen 2 Main Objekten funktioniert bin ich mir nicht sicher

Gruß Helmut

 

[Admiral Helmut]

ich glaube du brauchst eher so etwas

public class Database{
   private int count;
   public Database(){
   count = 0;
}
   public synchronized void increment(){
   count++;
   System.out.println(count);
}


}

public class Incrementer implements Runnable{
 Database d;
 public Incrementer(Database d){
   this.d = d;
}

public void run(){
for(int i = 0; i<50; i++)
d.increment();
Thread.sleep(1000);
}
}

public class Main{
   public static void main(String[] args){
Database d = new Database();
 Thread t1 = new Thread(new Incrementer(d));
Thread t2 =new Thread(new Incrementer(d));
t1.start();
t2.start();
}
    


}

 

[Shams]

Ich habe nun zwei Klassen, wie folgt (vielen Dank an Admiral Helmut, ich denke, dass das, was ich in der Zwischenzeit gemacht habe, dasselbe is ):

import java.io.*;

public class Main implements Runnable//implements Runnable
{
	TestClass tc;
	public Main(TestClass t){
		super();
		tc=t;
		
		
	}

	@Override
	public void run() {		
		tc.increment();
	  }
 
	
	public static void main(String[] args) 
	{
		TestClass tc = new TestClass();
		
		Main m = new Main(tc);

		Thread t1 = new Thread(m);
		Thread t2 = new Thread(m);
		t1.start();
		t2.start();

    }	 
}

und

public class TestClass {

public int count = 0; //Ob dies static ist, ist für synchronized egal.
	
	public int getCount() {
		return count;
	}

	public void setCount(int z){
		count=z;
	}
	 
	public synchronized void increment(){
		
		while(count<66){
		
        int zahl = getCount();
         zahl++;
        setCount(zahl);
         System.out.println(zahl);
     
		} 
		
	}
	
}

...ich habe es jetzt auch mal laufen lassen, und es werden die Zahlen von 1-66 ausgegeben.

 

[Admiral Helmut]

Das sieht vom Aufbau schon gut aus, die Synchronisierung in EIN Datenobject verlagert. Damit ist es Threadsafe.

Erklär mir und dir bitte den SInn der While schleife in deinem Programm.

dein erster Thread geht in die Synchronised Methode und sperrt sie.
dann geht er in die While Schleife. incrementiert 1, dann 2 ... er bleibt solange in der while schleife bist er bei 66 ist.

Achtung Methode immernoch gesperrt.

jetzt geht er aus der while schleife raus und gibt die Methode frei.
Jetzt darf Thread 2 zum ersten Mal in die Methode.
Thread 2:"Ohhh Count ist ja schon 66. Dann geh ich gar nicht in die While schleife". Programm Ende.

Nur Thread 1 gibt die Zahlen 1-66 aus

Ich denke da du dich mit Threads beschäftigst willst du abwechselnd incrementieren.

Ersetze deine While schleife mal durch eine if() Abfrage

if(count<66){
...increment....

}

Gruß Helmut

 

[Bug Fisher]

Am Ende ist es egal, es war von Anfang an threadsafe !

public synchronized void foo() {/*code*/}

ist eine Abkürzung für

public void foo() {
     synchronized(this) {/*code*/}
}

Während

public static synchronized void foo() {/*code*/}

zu

public static void foo() {
     synchronized(<Klassenname>.class) {/*code*/}
}

wird.

Beide Varianten sind aber aus OO - Sicht ein Designfehler von Java und sollten vermieden werden, denn Synchronisationsvariablen sind auf die Art nicht privat zu machen.

Bsp.:

class Broken {
     public synchronized void method() {}
}

class UserOfBroken {
     Broken b = new Broken();

     void method() {
          synchronized(b) {/*pitfall*/}
     }
}

Das Szenario lässt sich analog für die static Variante bauen, da ist es ja sogar noch viel einfacher, ein

synchronized(Broken.class)

zu verbrechen.

Synchronisationsobjekte sollten private Member sein, ich nutze dabei meist das private Threadobjekt selbst, wenn es es gibt, ansonsten einfach ein

final Object monitor = new Object();

 

[Shams]

Dies ist dann eventuell die finale Version

public class TestClass {
 
public int count = 0;  



 
	public int getCount() {
		return count;
	}
 
	public void setCount(int z){
		count=z;
	}
 
	public synchronized void increment(){
 
 
        int zahl = getCount();
        
        zahl++;
        setCount(zahl);
        
           System.out.println(zahl);
 
 
	}
 
}


////////////////////////////////////////////////////////////

import java.io.*;
 
public class Main implements Runnable//implements Runnable
{
	TestClass tc;
	public Main(TestClass t){
		super();
		tc=t;
 
 
	}
 
	@Override
	public void run() {		
		tc.increment();
	  }
 
 
	public static void main(String[] args) 
	{
		TestClass tc = new TestClass();
 
		Main m = new Main(tc);
 
		
int x = 0;
while(x<20){
	
		Thread t1 = new Thread(m);
		Thread t2 = new Thread(m);

                t1.start();
		        t2.start();
x++;
}

//Ausgabe 1234567....
 
    }	 
}

 

[Admiral Helmut]

Hallo Bug Fisher,
kurze Frage an dich:
Ich verstehe deine Nachricht aber bist du dir sicher, dass es von anfang an threadsafe war:
Er synchronisiert ja eine ObjectMethode und macht nur den Counter static. Dadurch dürften doch mehrere Objekte zur gleichen Zeit zugreifen.
Das mit dem static synchronized (bzw über die Klasse eben) habe ich jetzt glaub ich verstanden. aber seine Methode ist oben ja nicht static

Gruß Helmut

 

[Bug Fisher]

Hallo Helmut, ja du hast recht, ich habe mich geirrt, sorry.
Habe nochmal genau hingesehen.

Jeder Thread nutzt im ersten Post dieses Threads einen anderen (Main.this) Monitor.