Synchronized klappt nicht

[Mortal_Shadow]

Hi,

ich bin am rumprobieren mit synchronisation gewesen und dabei auf folgendes Problem gestosen:

public class Speicherproblem {
	
	int counter = 0;

	public static void main(String[] args) {
		Speicherproblem s = new Speicherproblem();
		s.test();
		
	}
	
	public void test(){
		new Thread(new Inkrementor()).start();
		new Inkrementor().run();
		System.out.println(counter);
	}
	
	public synchronized void add(){
		counter++;
	}
	
	public class Inkrementor implements Runnable{		
			@Override
			public void run() {
				for(int i = 1; i <= 10000; i++){
					add();
				}
			}
			
			
			
		}	
	
	
}

Ich erstelle hierbei zwei Inkrementoren, die in verschiedenen Threads counter um eins erhöhen.
Auch wenn sie dies in einer synchronized Methode machen, kommt als result ein zufälliger Wert zwischen
10.000 und 20.000 herraus.
Warum, wie macht mans richtig?

 

[jonius]

Ich würde sagen, du musst erst die Runnable-Instanz an den Thread übergeben und bei DIESEM die start()-Methode aufrufen.

 

[Atze]

hm, hab jetzt hier am lappi kein eclipse zum testen, aber rein vom gefühl her sieht das n bischen merkwürdig aus. hast du dich vorher mal eigehend mit dem thema concurrency / threads beschäftigt? was soll denn überhaupt rauskommen? 20.000?

also was so auffällt beim drüberschauen:

1. zeile 3 - vielleicht volatile setzen
2. zeilen 12 & 13 - warum die verschiedenen aufrufe? und niemals run() aufrufen, nur start(), sonst ist es eine simple methode, kein multithreading
3. die nested class nicht public machen, sonst braucht sie ne eigene datei. kompiliert dass denn so eigentlich? *grübel*

würd grundsätzlich nochmal über den code nachdenken

 

[andiv]

	public void test(){
		new Thread(new Inkrementor()).start();
		new Inkrementor().run();
		System.out.println(counter);
	}

Dein Problem entsteht durch diese Methode. Wenn die Zeile mit dem println ausgeführt wird, dann ist der eine Inkrementor fertig, d.h. counter ist mindestens 10000. Der Fortschritt des parallelen Threads ist aber völlig unklar, er kann zwischen 0 (kam noch nicht zum Zug) und 10000 (ist bereits fertig) liegen.

Funktionieren müsste es so (habs nicht getestet)

	public void test(){
		Thread t = new Thread(new Inkrementor());
		t.start();
		new Inkrementor().run();
		t.join();
		System.out.println(counter);
	}

 

[Mortal_Shadow]

Ich habe jetzt counter volatile gemacht, bringt nichts.

Zu run() vs start().
Schaut nochmal hin, das run() bezieht sich lediglich auf die Klasse Inkrementor, welche von Runnable abgeleitet ist.
Funktionieren tut das so:
Ich starte einen neuen Thread der dann anfängt counter zu erhöhen.
Derweil erhöhe ich im Ursprungsthread counter ebenfalls (new Inkrementor.run()).

Die nested class ist schlecht mit public, sehe ich ein, habe das auch sofort private gemacht.
Auswirkungen auf das Ergebnis hatte dies aber natürlich nicht.


Edit: Es war das fehlende .join().
Danke.

 

[Atze]

stimmt andiv, die ausgabe hatte auch garnicht auf die threads gewartet hab ich übersehen

 

[andiv]

1. zeile 3 - vielleicht volatile setzen

volatile ist hier nicht nötig da nur aus synchronized-Blöcken auf counter zugegriffen wird.

2. zeilen 12 & 13 - warum die verschiedenen aufrufe? und niemals run() aufrufen, nur start(), sonst ist es eine simple methode, kein multithreading

Der direkte Aufruf von run ist hier gewollt, zusammen mit dem Thread der die Methode ausführt sind dann nämlich 2 Threads am Werk.

3. die nested class nicht public machen, sonst braucht sie ne eigene datei. kompiliert dass denn so eigentlich? *grübel*

Eine nested class kann gar nicht in eine eigene Datei, dann wäre sie nämlich nicht mehr nested. public ist absolut in Ordnung hier.

Edit: zu langsam ;-)

 

[fastjack]

    new Thread(new Inkrementor()).start();
    new Inkrementor().run();

damit erstellt Du nicht zwei verschiedene Threads, sondern nur einen, richtig wäre:

    new Thread(new Inkrementor()).start();
    new Thread(new Inkrementor()).start();

jetzt sind es zwei Threads.

 

[Atze]

ja, ich hätte sie ja auch dann nicht nested gemacht egal, problem gelöst

 

[andiv]

@fastjack: Damit hättest du 3 Threads wenn du den Thread der die Methode ausführt mitzählst.

 

[fastjack]

Das meine ich nicht, der Fakt ist, das das hier:

new Inkrementor().run();

alleine überhaupt nicht ausreicht, einen Thread zu erzeugen. Es erzeugt lediglich ein Runnable-Objekt, dessen run-Methode Du aufrufst.

 

[fastjack]

@andiv wenn Du den Main-Thread mitzählst ja, sonst nein.

edit-- wie du auch festgestellt hast.