changeset 2727:499059dee241

Add synchronization to VmListenerBackend The backends will have a race condition in a later patch when there are multiple threads calling activate and deactive, of which this adds synchronization so the async activation/deactivation do not have a race condition. The race condition occurs when two threads enter the deactivation() block for unregistering; they both trigger the code that removes the backend from a list when it should only be removed once (which in turn causes an exception). Reviewed-by: jerboaa, aazores Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2017-July/024255.html
author Christopher Koehler <chkoehle@redhat.com>
date Tue, 25 Jul 2017 08:25:21 -0400
parents 2b8ca1e3186d
children e80359f05ea7
files plugins/jvm-overview/agent/src/main/java/com/redhat/thermostat/jvm/overview/agent/VmListenerBackend.java
diffstat 1 files changed, 21 insertions(+), 14 deletions(-) [+]
line wrap: on
line diff
--- a/plugins/jvm-overview/agent/src/main/java/com/redhat/thermostat/jvm/overview/agent/VmListenerBackend.java	Mon Jul 24 12:29:44 2017 -0400
+++ b/plugins/jvm-overview/agent/src/main/java/com/redhat/thermostat/jvm/overview/agent/VmListenerBackend.java	Tue Jul 25 08:25:21 2017 -0400
@@ -56,7 +56,8 @@
 public abstract class VmListenerBackend extends BaseBackend implements VmStatusListener {
     
     private static final Logger logger = LoggingUtils.getLogger(VmListenerBackend.class);
-    
+
+    private final Object activationLock = new Object();
     private WriterID writerId;
     private VmStatusListenerRegistrar registrar;
     private VmMonitor monitor;
@@ -95,11 +96,13 @@
      */
     @Override
     public boolean activate() {
-        if (!initialized) {
-            logger.warning("Backend not started, initialize must be called before activate");
-        } else if (!started && monitor != null) {
-            registrar.register(this);
-            started = true;
+        synchronized (activationLock) {
+            if (!initialized) {
+                logger.warning("Backend not started, initialize must be called before activate");
+            } else if (!started && monitor != null) {
+                registrar.register(this);
+                started = true;
+            }
         }
         return started;
     }
@@ -114,10 +117,12 @@
      */
     @Override
     public boolean deactivate() {
-        if (started && monitor != null) {
-            registrar.unregister(this);
-            monitor.removeVmListeners();
-            started = false;
+        synchronized (activationLock) {
+            if (started && monitor != null) {
+                registrar.unregister(this);
+                monitor.removeVmListeners();
+                started = false;
+            }
         }
         return !started;
     }
@@ -163,10 +168,12 @@
      * @param version version number of this backend
      */
     protected void initialize(WriterID writerId, VmStatusListenerRegistrar registrar, String version) {
-        this.writerId = writerId;
-        this.registrar = registrar;
-        setVersion(version);
-        this.initialized = true;
+        synchronized (activationLock) {
+            this.writerId = writerId;
+            this.registrar = registrar;
+            setVersion(version);
+            this.initialized = true;
+        }
     }
     
     /**