From: Tuukka Lehtonen <tuukka.lehtonen@semantum.fi>
Date: Mon, 16 Mar 2020 22:03:55 +0000 (+0200)
Subject: Fixed ProfileObserver.update race with multiple query threads
X-Git-Url: https://gerrit.simantics.org/r/gitweb?p=simantics%2Fplatform.git;a=commitdiff_plain;h=e83110633e844749640f830e186423643e1b7cbe

Fixed ProfileObserver.update race with multiple query threads

gitlab #499

Change-Id: I79fcc0c67e6ac2850e8c6949e499219b5c43cfcf
---

diff --git a/bundles/org.simantics.scenegraph.profile/META-INF/MANIFEST.MF b/bundles/org.simantics.scenegraph.profile/META-INF/MANIFEST.MF
index e032ff417..6d8c271b0 100644
--- a/bundles/org.simantics.scenegraph.profile/META-INF/MANIFEST.MF
+++ b/bundles/org.simantics.scenegraph.profile/META-INF/MANIFEST.MF
@@ -14,7 +14,7 @@ Require-Bundle: org.simantics.db.layer0;bundle-version="1.1.0",
  org.simantics.scenegraph;bundle-version="1.1.1",
  org.eclipse.core.runtime;bundle-version="3.6.0",
  org.simantics.diagram.ontology;bundle-version="1.1.1",
- org.simantics.db.common
+ org.slf4j.api
 Bundle-ActivationPolicy: lazy
 Bundle-Activator: org.simantics.scenegraph.profile.impl.Activator
 Import-Package: org.simantics
diff --git a/bundles/org.simantics.scenegraph.profile/src/org/simantics/scenegraph/profile/common/ProfileObserver.java b/bundles/org.simantics.scenegraph.profile/src/org/simantics/scenegraph/profile/common/ProfileObserver.java
index 9d379c890..440ee2714 100644
--- a/bundles/org.simantics.scenegraph.profile/src/org/simantics/scenegraph/profile/common/ProfileObserver.java
+++ b/bundles/org.simantics.scenegraph.profile/src/org/simantics/scenegraph/profile/common/ProfileObserver.java
@@ -23,8 +23,8 @@ import org.simantics.db.AsyncRequestProcessor;
 import org.simantics.db.Resource;
 import org.simantics.db.Session;
 import org.simantics.db.common.session.SessionEventListenerAdapter;
-import org.simantics.db.common.utils.Logger;
 import org.simantics.db.procedure.Procedure;
+import org.simantics.db.service.QueryControl;
 import org.simantics.db.service.SessionEventSupport;
 import org.simantics.scenegraph.INode;
 import org.simantics.scenegraph.g2d.G2DSceneGraph;
@@ -38,9 +38,13 @@ import org.simantics.utils.datastructures.Pair;
 import org.simantics.utils.datastructures.disposable.IDisposable;
 import org.simantics.utils.threads.IThreadWorkQueue;
 import org.simantics.utils.threads.ThreadUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class ProfileObserver implements EvaluationContext {
 
+    private static final Logger LOGGER = LoggerFactory.getLogger(ProfileObserver.class);
+
     private final Session                     session;
 
     /**
@@ -58,6 +62,7 @@ public class ProfileObserver implements EvaluationContext {
     private volatile boolean                  dirty               = true;
     private volatile boolean                  disposed            = false;
 
+    private boolean                           needSynchronizedUpdates = false;
     private List<Pair<Style, Object>>         updates             = new ArrayList<>();
     private boolean                           updateAll;
     
@@ -95,6 +100,7 @@ public class ProfileObserver implements EvaluationContext {
         this.sceneGraph = sceneGraph;
         this.constants.putAll(constants);
         this.notification = notification;
+        this.needSynchronizedUpdates = session.getService(QueryControl.class).getAmountOfQueryThreads() > 1;
 
         attachSessionListener();
 
@@ -139,8 +145,14 @@ public class ProfileObserver implements EvaluationContext {
     public void update(Style style, Object item) {
         if (DebugPolicy.DEBUG_PROFILE_OBSERVER_UPDATE)
             System.out.println("Profile observer marked dirty.");
-        
-        updates.add(Pair.make(style, item));
+
+        if (needSynchronizedUpdates) {
+            synchronized (updates) {
+                updates.add(Pair.make(style, item));
+            }
+        } else {
+            updates.add(Pair.make(style, item));
+        }
         //updateAll = true;
         dirty = true;
     }
@@ -179,10 +191,25 @@ public class ProfileObserver implements EvaluationContext {
                                 e.apply(ProfileObserver.this);
                             }
                             updateAll = false;
-                            updates.clear();
+                            if (needSynchronizedUpdates) {
+                                synchronized (updates) {
+                                    updates.clear();
+                                }
+                            } else {
+                                updates.clear();
+                            }
                         } else {
-                            List<Pair<Style, Object>> updatesCopy = new ArrayList<>(updates);
-                            updates.clear();
+                            List<Pair<Style, Object>> updatesCopy;
+                            if (needSynchronizedUpdates) {
+                                synchronized (updates) {
+                                    updatesCopy = new ArrayList<>(updates);
+                                    updates.clear();
+                                }
+                            } else {
+                                updatesCopy = new ArrayList<>(updates);
+                                updates.clear();
+                            }
+
                             for (Pair<Style, Object> update : updatesCopy) {
                                 Style style = update.first;
                                 Object item = update.second;
@@ -221,7 +248,7 @@ public class ProfileObserver implements EvaluationContext {
 
             @Override
             public void exception(Throwable t) {
-                Logger.defaultLogError(t);
+                LOGGER.error("RuntimeProfileActiveEntries request failed", t);
             }
         });
     }
@@ -233,7 +260,7 @@ public class ProfileObserver implements EvaluationContext {
 
     @Override
     public void exception(Throwable throwable) {
-        Logger.defaultLogError(throwable);
+        LOGGER.error("Exception occurred during diagram profile observation", throwable);
     }
 
     @SuppressWarnings("unchecked")