Singletons und Multithreading

[prolahmierer]

Hallo!

Ich habe einen Multi-Threaded Server mit n aktiven Verbindungen.

Und eine Klasse die Verbindungsdaten verwaltet, implementiert als Singleton:

public class PlayerLookup {

    private static PlayerLookup instance = new PlayerLookup();
    
    private HashMap<Socket, LookupAssoc> lookupTable;

    private PlayerLookup() {
	lookupTable = new HashMap<Socket, LookupAssoc>();;
    }

    public static PlayerLookup getInstance() {
	return instance;
    }

    // weitere methoden die auf der HashMap arbeiten folgen hier, z.b.:
    public void removeBySocket(Socket toRemove){
	lookupTable.remove(toRemove);
    }

Wenn neue Verbindungen dazu kommen oder Verbindungen verloren gehen wird das in dieser HashMap geloggt. Nur kriege ich da teilweise die merkwürdigsten Ereignisse.
Weis jemand, in wie weit diese Singleton-Implementierung Thread-Safe ist? Oder muss ich da noch Methoden synchronisieren. Eigentlich dachte ich, die Singleton-Implementierung würde alle Probleme beseitigen. Vielleicht liegt der Fehler auch woanders aber erstmal wollte ich das hier ausschließen.

Danke schon mal,
Prolahmierer

 

[maki]

[c]removeBySocket[/c] ist nicht Threadsafe, da HashMap eben nicht Threadssafe sind, alle Methoden die die Map ändern sollten synchronisiert werden.

 

[Marco13]

Doch, [c]removeBySocket[/c] ist """Thread-safe""", weil das die einzige Methode ist, die irgendwas mit der lookupTable macht

Aber davon ausgehend, dass es auch sowas wie "addBySocket" gibt: Dort braucht man synchronisation. Es würde wohl schon reichen, einfach alle Methoden, die auf die lookupTable zugreifen "synchronized" zu machen. Die synchronisation irgendwie bis zur lookupTable selbst reinzuziehen (irgendwie als Collections.synchronizedMap) macht IMHO dort keinen Sinn.

 

[prolahmierer]

Ok danke, dann liegt da wohl ein Denkfehler vor. Ich dachte durch das Singleton arbeiten alle auf dem selben Objekt und das würde dann "schon in Ordnung gehen". In der Tat gibt es noch weitere Methoden, ich wollte sie jetzt nicht alle hier reinkopieren.

Warum macht eine syncrhonizedMap noch keinen Sinn? Dann wäre doch ruhe und man müsste nicht auf alle Methoden achten.

Grüße

 

[maki]

Ok danke, dann liegt da wohl ein Denkfehler vor. Ich dachte durch das Singleton arbeiten alle auf dem selben Objekt und das würde dann "schon in Ordnung gehen". In der Tat gibt es noch weitere Methoden, ich wollte sie jetzt nicht alle hier reinkopieren.

Eben weil alle Threads auf dem selben Obejkt arbeiten "geht es nicht in Ordnung" ohne zusätzliche Synchronisation.

Warum macht eine syncrhonizedMap noch keinen Sinn?

imho weil Singletons eigentlich immer threadsicher gemacht werden müssen, liegt in der Natur der Sache..

 

[velaluka]

Hallo prolahmierer,
oder mal testen wie sich die Applikation verhält wenn du die HashMap als volatile definiertst.
Java: Datenzugriffe mit dem Schlüsselwort <i>volatile</i> synchronisieren - Seite 1 von 1 - Anwendungsentwicklung | Developer | ZDNet.de... mal als Einstieg zum Googlen.
Ciao Velaluka

 

[prolahmierer]

Danke! Das Volatile sieht auch interessant aus, werde ich mal testen.

Noch eine kleine Frage:
Um die eingehenden Daten synchron an eine Klasse zu schicken die die Daten dann bearbeitet, habe ich mir eine BlockingQueue-Klasse gebaut, die die Daten in eine BlockingQueue einreiht. Die Queue läuft als Thread und benachrichtigt bei eingehenden Nachrichten die Bearbeiterklasse:

Kann ich dann davon ausgehen, dass die Daten der Reihe nach vom Bearbeiter abgearbeitet werden und nciht mehr parallel?

public class GameQueue extends Thread {

    private static GameQueue instance = new GameQueue();
    private BlockingQueue<QueueObject> queue = new LinkedBlockingQueue<QueueObject>();

    private GameQueue() {
	this.setName("Thread-GameQueue");
	start();
    }
    public static GameQueue getInstance() {
	return instance;
    }

    public void run() {
	try {
	    QueueObject inputobj;
	    while ((inputobj = queue.take()) != null) {
		Manager.execute(inputobj) // verkürzt, kommt aber aufs gleiche raus
	    }

	} catch (InterruptedException e) {

	}
    }

 

[prolahmierer]

Ok vergesst das mit der Queue. Ich hab alle Zugriffe auf die Hashmap synchronisiert, dann geht es (anscheinend ) Die Queue-Zugriffe habe ich auch synchronisiert.

Noch eine Frage:
Warum reicht es nicht nur schreib-zugriffe zu synchronisieren? Wenn ich die Lese-Zugriffe nicht synhchronisiere treten wieder merkwürdige Erscheinungen ans Tageslicht.
Hat das damit zu tun, das er gleichzeitig von Daten liest, die währenddessen verändert werden?

 

[Atze]

jop

synchronized read(){}
synchronized write(){}

heißt, das beide methoden eines objekt nur von einem thread durchlaufen werden können, sie sind implizit beide an this sychronisiert.

wenn nur write() sync wäre, kann thread B lesen, was A gerade schreibt. bei sync von beiden darf B erst in read, wenn A aus dem lock ist, also hast du recht

 

[prolahmierer]

Danke, Atze.

 

[Spacerat]

Mal abgesehen von der HashMap... was könnte denn bei Multithreading mit Singletons noch so alles schief gehen? Z.B. die Instanzierung des Singletons?

public final class Singleton
{
  private static final Singleton instance;

  static
  {
    instance = new Singleton();
  }

  private Singleton()
  {
    //...
  }

  public static Singleton getInstance()
  {
    return instance;
  }
}

 

[Marco13]

Da hab' ich auch extra nachgeschaut, aber so, wie es im Moment gemacht ist, passt das schon