nochmal synchronisierte Methoden

[El Hadji]

Servus Community,
Ich hab hier ein kleines Problem und komm nicht mehr weiter. Was sagt ihr zu meinem Code bis jetzt. Folgende Angabe:

Hier mein Code bis jetzt:

package threads;

import threads.geg.Receiver;
import java.util.*;

public class Communicator implements Receiver
{
   
    private HashMap <String, Receiver> liste;
    private static  Object lock = new Object();
    private static Object lock2 = new Object();
    private int zaehler;
    private String name;
    private int send;
    private int sendAll;
  
    public Communicator()
    {
       liste = new HashMap<String,Receiver>();
       zaehler = 0;
       name = "Receiver";
       send = 0;
       sendAll =0;
    }

 
    public void receive(String message)
    {
      System.out.println(message);
    }
   
    public synchronized String signUp(Receiver aReceiver)
    {
        if(liste.containsValue(aReceiver))
        {
            return name;
        }
       
        zaehler++;
        name = "Receiver" + zaehler;
        liste.put(name, aReceiver);
        return name;
      
    }
   
    public void send(String identifier, String message)
    {
       synchronized (lock)
       {
           while(sendAll > 0)
           {
               try
               {
               wait();
               send--;
            }
           
            catch(InterruptedException e)
            {
                throw new RuntimeException(e);
            }
            }
           
            if(!liste.containsKey(identifier))
            {
                throw new RuntimeException("Invalid identifier");
            }
           
            liste.get(identifier).receive(message);
            send++;
            this.notifyAll();
        }
      
    }

   
   
    public synchronized void sendAll(String message)
    {
      
       
        while(send > 0)
        {
            try
            {
                wait();
                sendAll--;
            }
            catch(InterruptedException e)
            {
                throw new RuntimeException(e);
            }
        }
       
        Set<String> neu = liste.keySet();
       
        for(String f : neu)
        {
            liste.get(f).receive(message);
        }
        sendAll++;
        this.notifyAll();
    }
   
   
}

Ich würde mich über eine Antwort freuen
mfg El Hadji

 

[httpdigest]

Nein. Das ist leider komplett falsch. Schau dir doch nurmal die signUp() Methode an. Hier würdest du ja immer den Namen des zuletzt hinzugefügten Receivers zurückgeben, wenn versucht wird, irgendeinen Receiver, der bereits registriert ist, erneut zu registrieren. Du solltest dir schon ein Mapping von Receiver auf String merken.
Dann ist das Locking einmal zu restriktiv und zweitens völlig falsch. Aktuell darf nur ein einziger Thread überhaupt an irgendeinen Receiver per send(String identifier, String message) senden, da - egal welcher Receiver - immer über dasselbe lock Objekt synchronisiert wird.
Dann ist die Prüfung von sendAll innerhalb von send() nicht korrekt mit dem Aktualisieren dieses Zählers in sendAll() synchronisiert. Der Zähler kann völlig verschränkt zueinander aktualisiert und gelesen werden. Wenn auf gemeinsame Variablen zugegriffen wird, muss IMMER über dasselbe Lock-Objekt synchronisiert werden. sendAll() synchronisiert hier über this während send() über das lock Objekt synchronisiert.
Außerdem muss du darauf achten, dass alle Zugriffe auf die "liste" korrekt synchronisiert sind, wenn mehrere Threads gleichzeitig send(), sendAll() und/oder signUp() aufrufen.
Das, was du machen sollst, ist alles andere als einfach, lässt sich aber vorzüglich mittels der ReadWriteLock/ReentrantReadWriteLock Klasse vom JRE realisieren - falls du das nutzen darfst.

 

[El Hadji]

Ok ich wollte natürlich auch die Receiver als Key nehmen, aber die Methode send will ja den String als identifier haben. Sprich ich übergebe den Value-Wert und da könnte ich ja eine Nachricht an 2 Receivern gleichzeitig schicken falls die eben den gleichen Value haben.

Ah ok ich dachte mit dem lock, verhindere ich dass die send Methode an 2 gleiche Receiver gleichzeitig sendet. Also besser ein synchronized(this).

Ja mit dem Zähler des SendAll() war mir klar, wusste aber nicht wie ich das verhindern kann.

Leider eben nicht, wir dürfen nur eine selbstgeschrieben CheckAndSet-Methode verwenden

 

[httpdigest]

Ok ich wollte natürlich auch die Receiver als Key nehmen, aber die Methode send will ja den String als identifier haben. Sprich ich übergebe den Value-Wert und da könnte ich ja eine Nachricht an 2 Receivern gleichzeitig schicken falls die eben den gleichen Value haben.

Du musst natürlich sicherstellen, dass nicht zwei Receiver denselben Identifier/Schlüssel bekommen. Das ist ja klar, denn dann weißt du ja in send() nicht, welchem Receiver du nun die Nachricht schicken sollst.

Ah ok ich dachte mit dem lock, verhindere ich dass die send Methode an 2 gleiche Receiver gleichzeitig sendet. Also besser ein synchronized(this).

Nein, die Synchronisation auf dasselbe Lockobjekt (ob das nun `lock` ist oder `this` ist völlig wurscht) verhindert doch, dass du überhaupt an zwei oder mehr Receiver eine Nachricht gleichzeitig senden kannst.

 

[httpdigest]

Hier ist eine (wahrscheinlich) funktionierende Lösung (habe die Funktionalität in viele kleine, leicht zu verstehende Methoden aufgeteilt):

Spoiler

import java.util.*;
import java.util.function.*;
import java.util.stream.*;
interface Receiver {
  void receive(String message);
}
public class Communicator {
  private final Map<Receiver, String> r2i = new HashMap<>();
  private final Map<String, Receiver> i2r = new HashMap<>();
  private int sequence;
  private int shared;
  private boolean exclusive;
  private <T> T doExclusively(Supplier<T> r)
      throws InterruptedException {
    lockExclusive();
    try {
      return r.get();
    } finally {
      unlockExclusive();
    }
  }
  private void lockExclusive() throws InterruptedException {
    synchronized (this) {
      while (shared > 0 || exclusive)
        wait();
      exclusive = true;
    }
  }
  private void unlockExclusive() {
    synchronized (this) {
      exclusive = false;
      notifyAll();
    }
  }
  private void doShared(Runnable r) throws InterruptedException {
    lockShared();
    try {
      r.run();
    } finally {
      unlockShared();
    }
  }
  private void lockShared() throws InterruptedException {
    synchronized (this) {
      while (exclusive)
        wait();
      shared++;
    }
  }
  private void unlockShared() {
    synchronized (this) {
      shared--;
      notifyAll();
    }
  }
  public String signUp(Receiver receiver) throws InterruptedException {
    return doExclusively(() -> r2i.computeIfAbsent(receiver, (r) -> {
      String someIdentifier = Integer.toString(sequence++);
      i2r.put(someIdentifier, receiver);
      return someIdentifier;
    }));
  }
  public void send(String identifier, String message)
      throws InterruptedException {
    Receiver receiver = doExclusively(() -> {
      if (!i2r.containsKey(identifier))
        throw new RuntimeException("Invalid identifier!");
      return i2r.get(identifier);
    });
    doShared(() -> {
      synchronized (receiver) {
        receiver.receive(message);
      }
    });
  }
  public void sendAll(String message) throws InterruptedException {
    doExclusively(() -> {
      r2i.keySet().forEach(r -> r.receive(message));
      return null;
    });
  }
  public static void main(String[] args) throws InterruptedException {
    Communicator c = new Communicator();
    String id1 = c.signUp((msg) -> System.out.println("1:" + msg));
    String id2 = c.signUp((msg) -> System.out.println("2:" + msg));
    IntStream.range(0, 1000).parallel().forEach(i -> {
      try {
        if ((i % 2) == 0)
          c.sendAll("to all");
        else if ((i % 3) == 0)
          c.send(id1, "to one");
        else
          c.send(id2, "to two");
      } catch (InterruptedException e) {
        e.printStackTrace();
      }
    });
  }
}

 

[El Hadji]

Vielen Dank.
Aber dennoch braauch ich eine Zeit bis ich alle MEthoden verstehe 🙂