Threads MultiThreading NullPointerException

[TheCubeCrafter]

Hi Community,

ich habe mal wieder ein Problem mit meinem aktuellen Projekt. Ich versuche mich gerade in Mutithreading einzuarbeiten. Das synchronisieren der Threads funktioniert schonmal gut nur ich bekomme eine NullPointerException wenn ich mehrere Threads erstelle. Das komische ist, dass der Fehler nur manchmal kommt und in welchen Threads er kommt ist auch unterschiedlich. Mal in keinem mal in mehreren, aber mir ist noch nie aufgefallen dass er in allen kommt.

Hier erstelle ich die Threads in der Main Methode:

Rechner[] r = new Rechner[4];
Thread[] rt = new Thread[4];
		
for(int i = 0; i < rt.length; i++){
	r[i] = new Rechner(primes);

	rt[i] = new Thread(r[i]);
	rt[i].start();
}

Und hier bekomme ich den Fehler in Zeile 30 (Fett geschriebene Zeile):

package de.Jonas.Primzahlenrechner;

import java.util.ArrayList;
import java.util.List;

public class Rechner implements Runnable {

	private long numberToCalculate = 0;
	private long numberToDivide = 0;
	
	private List<Long> primes = new ArrayList<Long>();

	public Rechner(List<Long> primes){
		this.primes = primes;
	}
	
	
	@Override
	public void run() {
		
		while(true){			
			
			numberToCalculate = Primzahlenrechner.getActNumber();
			numberToDivide = (numberToCalculate / 2) + 1;
						
			if(this.numberToCalculate % 2 != 0){
				boolean b = true;
								
				for(int i = 0; i < this.primes.size(); i++){
					[B]if(this.primes.get(i) < this.numberToDivide){[/B]
						if(this.numberToCalculate % this.primes.get(i) == 0){
							b = false;
							break;
						}
					}
				}
				
				if(b == true){
					Primzahlenrechner.addPrime(numberToCalculate);
				}
			}
		}
	}
}

Nur das komische daran ist, dass ich eine Zeile darüber noch auf das Objekt zugreifen kann ohne dass ich eine Exception bekomme. Wenn ich in Zeile 25 noch ein System.out.println(" "); einfüge läuft alles ohne probleme nur das möchte ich nicht da es eigentlich eine konsolenanwendung bleiben sollte.

Kann mir vielleicht jemand dabei helfen?

Grüße

Jonas

 

[Bug Fisher]

Das ist zwar nicht die Ursache aber Quatsch wie du sicher selbst siehst.

private List<Long> primes = new ArrayList<Long>();
 
public Rechner(List<Long> primes){
    this.primes = primes;
}

so sollte es sein, nicht ?

private final List<Long> primes;
 
public Rechner(List<Long> primes){
    this.primes = primes;
}

Der Fehler kann eigentlich nur daher kommen, dass die 'primes' Liste nicht korrekt synchronisiert ist.
Entweder du baust das selbst, nutzt Collections.synchronizedList(), oder, was ich am schönsten finde:

Freunde dich mit Callable<> an, lasse jeden Thread eine höchsteigene Liste zurückgeben, und führe am Ende alles zusammen. -> Thread.join()

Noch highleveliger wäre das Nutzen eines Threadpool mit Hilfe von zB. ExecutorService.newFixedThreadPool()
und der Future<> Klasse.

Oh, ich sehe gerade, dass

Primzahlenrechner.getActNumber()

auch konkurrierend aufgerufen wird; ist das ebenfalls korrekt synchronisiert ?

 

[TheCubeCrafter]

Das ist zwar nicht die Ursache aber Quatsch wie du sicher selbst siehst.

    private List<Long> primes = new ArrayList<Long>();
     
    public Rechner(List<Long> primes){
    this.primes = primes;
    }

so sollte es sein, nicht ?

    private final List<Long> primes;
     
    public Rechner(List<Long> primes){
    this.primes = primes;
    }

Ja genau. Das hab ich wohl vergessen wieder zu ändern.


Ich habe alle Zugriffe auf die Liste synchronisiert indem ich die Methoden mit synchronized versehen habe. (Also public static synchronized void...)


Mit Callable wäre es möglich. Allerdings habe ich momentan eine weitere Klasse die auf Eingaben wartet. Diese kann dann unter anderem alle errechneten Primzahlen ausgeben. Das wäre dann doch nur möglich wenn ich jeden Thread nur kleine Zahlenräume rechnen lasse (z.B. 100 Zahlen). Außerdem benötigt bei meiner Rechenmethode jeder Thread jede Primzahl die kleiner oder gleich der hälfte der aktuellen Zahl ist. Das wäre dann doch nicht möglich weil nicht auf alle errechneten Zahlen zugegriffen werden kann, oder?
Wäre es dann nicht besser wenn man jeden Thread nur eine Zahl errechnen lässt und dann jeweils für jeden fertigen Thread einen neuen startet?


Das:

Primzahlenrechner.getActNumber()

habe ich genauso synchronisiert wie die Methoden die auf 'primes' zugreifen. Das sollte auch funktionieren, da ich bis jetzt noch keine Primzahl mehrfach in der Liste gefunden habe.

 

[Bug Fisher]

Hmm, wirklich ne NullPointer... wo, wann und in welchem Thread erstellst du denn die Liste, die du an alle Rechner-Instanzen weitergibst ?

Mit Callable wäre es möglich. Allerdings habe ich momentan eine weitere Klasse die auf Eingaben wartet. Diese kann dann unter anderem alle errechneten Primzahlen ausgeben. Das wäre dann doch nur möglich wenn ich jeden Thread nur kleine Zahlenräume rechnen lasse (z.B. 100 Zahlen).

Ja, ich habe grad gesehen, dass du quasi endlos reinpackst, bis du das alles abbrichst.
Das könnte man ja ändern, dass du immer zB in 1000er chunks oder so erhöst.

Für die Ergebnisse zum Anzeigen, böte sich eine ArrayBlockingQueue<> an, so liesse sich das leicht synchronisieren; die Queue hat ne Obergrenze, und das Reinpacken blockiert dann so lange, bis wieder Platz ist. Das Rausnehmen blockiert, wenn nichts zu holen ist.

BTW: Dein Algorithmus hat einen Denkfehler: Du brauchst NICHT die Primzahlen bis n/2 sondern nur bis sqrt(n). Lässt sich aus der Primfaktorzerlegung ableiten.

Du könntest statt dessen auch das Sieb des Erathosthenes anwenden und wiederrum jedem Thread eine eigene Liste erstellen, die nur der eine Thread modifiziert. Diese fertige Teilliste könnte er dann dem Thread in die Queue packen,der das anzeigen übernimmt. Dass du je mehr Performance durch Overhead einbüsst, je mehr du synchronisierst sollte dir klar sein. Bei grossen Primzahlen ist das wichtig.

 

[TheCubeCrafter]

Ja es ist leider wirklich eine NullPointerException. Die Liste erstelle ich direkt vorher. Also ebenfalls in der Main Methode.
Mir ist gerade noch aufgefallen dass ich beim posten wohl was falsch gemacht habe. Der Fehler tritt nicht in Zeile 30 sondern in Zeile 31 auf. Da ist ganz oben wohl noch eine Zeile mit dazu gekommen als ich den Code kopiert habe. Eine Zeile davor greift er noch ohne Probleme auf die Größe der Liste zu. Und der Fehler kommt wie gesagt nicht in allen Threads wenn er denn überhaupt kommt.

Mir ist gerade aufgefallen, dass wenn ich das immer in Blöcken berechnen lasse es doch nicht möglich ist bei den kleineren Zahlen die Primzahlen richtig berechnen zu lassen. Denn wenn ich z.B. zwei Threads starte (einer von 2 - 1000 der andere von 1001 - 2000) ist die Liste mit allen Primzahlen noch leer. Also würde der Algorithmus in dem zweiten Thread nicht richtig rechnen, da er keine Liste mit den kleinen Primzahlen hat. Somit wären laut dem Algorithmus alle Zahlen Primzahlen.

Das mit der Wurzel von n hört sich gut an. Das werde ich gleich mal abändern. Das Sieb des Erathosthenes braucht aber glaube ich mehr Rechenzeit als mein Algorithmus da man auch jedes Vielfaches markieren muss.

 

[TheCubeCrafter]

Ich habe das Problem jetzt selber gelöst! . Ich habe jetzt das:

this.primes = new ArrayList<Long>(Primzahlenrechner.getPrimes());

in die while schleife gepackt und dafür den Construktor gelöscht.
Damit erstellt sich jeder Thread eine eigene Liste in der alle Primzahlen stehen die unabhängig von der Hauptliste ist. Damit läuft er jetzt zwar nicht mehr so schnell wie vorher aber ich bekomme keine NullPointerException mehr.

Danke für deine Hilfe.

 

[Bug Fisher]

Damit erstellt sich jeder Thread eine eigene Liste in der alle Primzahlen stehen die unabhängig von der Hauptliste ist. Damit läuft er jetzt zwar nicht mehr so schnell wie vorher aber ich bekomme keine NullPointerException mehr.

Das klingt erstmal gut - wenn es so jetzt aber erst geht, aber ich bin mir sicher, dass du da irgendwo an anderer Stelle was falsch machst, denn primes wäre der einzige Kandidat für eine NPE gewesen (auch in den anderen Zeilen der run()) und kann nur versehentlich genullt werden - wenn du dem jetzt doch noch auf den Grund gehen willst, würde ich dir raten, mit dem debugger mal den "Weg" dieser Liste primes Schritt für Schritt nachzuvollziehen. Auftreten wird der Fehler wärend des debuggens dann zwar ziemlich sicher nicht - dazu musst du dir dann an jeder Stelle überlegen, "was wenn jetzt genau hier ein anderer Thread..."

Das Sieb des Erathosthenes braucht aber glaube ich mehr Rechenzeit als mein Algorithmus

Das stimmt nicht. Aber das kannst du selbst nachlesen.

 

[TheCubeCrafter]

Das stimmt nicht. Aber das kannst du selbst nachlesen.

Da habe ich wohl zuerst was falsch verstanden. Jetzt habe ich den Pseudocode von Wikipedia mal in Java übersetzt und weis jetzt wie er funktioniert . Allerding bin ich mir sicher dass man den Algorhitmus nicht in Multithreading laufen lassen kann, da dieser Algorhitmus auf den ersten Zahlen aufbaut. Aber das ist jetzt auch egal da mein aktueller Code mehr als doppelt so schnell auf einem Prozessorkern läuft als der alte auf vier Kernen . Vielleicht lässt sich daran auch noch was optimieren.

Danke für deine Hilfe