Gibt es in diesem Code irgendwelche Thread-Sicherheitsfehler?

Question

Ich habe eine Singleton Service Implementierungsklasse mit Enum in meinem Java-Web-Anwendung. Es startet einmal beim Starten und Beenden der Anwendung, wenn die Anwendung nicht bereitgestellt wurde. und es eine Service-Methode Client bieten:

public enum SingletonService{ 
    INSTANCE; 
    private boolean isStarted; 

    public synchronized void start(){ 
    if(!isStarted){ 
     // do initialization stuff 
     isStarted = true; 
    } 
    } 

    public void stop(){ 
     checkStarted(); 
     // do stop jobs 
     isStarted = false; 
    } 

private synchronized void checkStarted(){ 
    if(!isStarted) 
     throw new RuntimeException("SingletonService is not ready"); 
} 

    public void service(){ 
     checkStarted(); 
     // do service job 
    } 

} 

Threading ein bisschen schwer für mich, ich bin beunruhigend, dass ich knifflige Fehler in meinem Code verpasst. Ist das notwendig, um start und checkStarted synchronisiert zu machen? Bitte sagen Sie mir etwas Schlechtes in einem solchen Code. Ich möchte auch wissen, ob es ein gemeinsames Muster dafür in Java gibt?

Answer 1

Zwei Kommentare mit Gründen:

1. checkStarted privat deklariert ist. Es ändert nicht den Zustand des Dienstes. Es muss nicht als synchronisiert deklariert werden, wenn Punkt 2 angesprochen wird.

2. Stop ist öffentlich, es ändert den Zustand des Dienstes. es sollte stattdessen als synchronisiert deklariert werden. Zwei Threads, die gleichzeitig auf stop/start zugreifen, sollten auf synchronisiertenStarted zugreifen.

   public enum SingletonService{ 
   INSTANCE; 
   private boolean isStarted; 
   
   public synchronized void start(){ 
   if(!isStarted){ 
       // do initialization stuff 
       isStarted = true; 
   } 
   } 
   
   public synchronized void stop(){ 
       checkStarted(); 
       // do stop jobs 
       isStarted = false; 
   } 
   
   private void checkStarted(){ 
       if(!isStarted) 
        throw new RuntimeException("SingletonService is not ready"); 
   } 
   
   public void service(){ 
       checkStarted(); 
       // do service job 
   } 
   
   } 
   

Answer 2

-Code ist Thread-sicher, wenn gewährleistet ist, wie erwartet verhalten, wenn sie von mehreren Threads auf einmal verwendet. Daher ist es schwer zu beurteilen, ob dieser bestimmte Code threadsicher ist und nicht weiß, was er erwartet. Aber ich werde versuchen und raten. Ich vermute, dass Sie unter anderem erwartet haben, dass wenn checkStarted() von INSTANCE.service() keine Ausnahme ausgelöst wird, die service() Methode in einer Annahme sicher gehen kann, dass Ihr Singleton nicht gestoppt wird, bevor service() seine Ausführung beendet. Mit dieser Annahme ist die Antwort: Ihr Code ist nicht Thread-sicher, Sie sollten zum Beispiel synchronize sowohl stop als auch service Methoden hinzufügen. Sie können dann synchronized auf checkStarted loswerden, wie von @ Guarava Agarwal Antwort erwähnt.

Betrachten Sie zwei Threads. Thread A ruft INSTANCE.start() gefolgt von INSTANCE.stop() und Thread B ruft INSTANCE.service(). Die folgende Sequenz von Ausführungen ist durch Ihre aktuelle Synchronisation nicht verboten:

1. Thread A führt INSTANCE.start() aus. isStarted ist auf true eingestellt.
2. Thread A gibt INSTANCE.stop() ein und führt INSTANCE.checkStarted() aus (keine Ausnahme ausgelöst).
3. Thread B gibt INSTANCE.service() ein und führt INSTANCE.checkStarted() aus (auch hier keine Ausnahme).
4. Gewinde A beendet INSTANCE.stop(), Einstellung isStarted auf false.
5. Thread B fährt mit seinem Service-Job fort, wobei Singleton bereits gestoppt wurde, entgegen unserer ursprünglichen Annahme.