From e83110633e844749640f830e186423643e1b7cbe Mon Sep 17 00:00:00 2001 From: Tuukka Lehtonen Date: Tue, 17 Mar 2020 00:03:55 +0200 Subject: [PATCH] Fixed ProfileObserver.update race with multiple query threads gitlab #499 Change-Id: I79fcc0c67e6ac2850e8c6949e499219b5c43cfcf --- .../META-INF/MANIFEST.MF | 2 +- .../profile/common/ProfileObserver.java | 43 +++++++++++++++---- 2 files changed, 36 insertions(+), 9 deletions(-) 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> 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> updatesCopy = new ArrayList<>(updates); - updates.clear(); + List> updatesCopy; + if (needSynchronizedUpdates) { + synchronized (updates) { + updatesCopy = new ArrayList<>(updates); + updates.clear(); + } + } else { + updatesCopy = new ArrayList<>(updates); + updates.clear(); + } + for (Pair 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") -- 2.47.1