Xcos: fix crash on block parameter change #2 24/20824/2
Clement DAVID [Fri, 8 Feb 2019 06:34:54 +0000 (07:34 +0100)]
After eca8e3d, on windows x64 Release build, Xcos keep crashing on block
update listener. Wrapping the (uid, kind) object into an owned
ScicosObjectOwner Java object will delegate the
referenceObject/deleteObject to the Java GC and will discard Java
statements reordering (thus the crash).

Change-Id: I979a324788265e32b60e75ad1ca60836d8e581aa

scilab/modules/xcos/src/java/org/scilab/modules/xcos/UpdateStyleFromInterfunctionAdapter.java
scilab/modules/xcos/src/java/org/scilab/modules/xcos/XcosView.java
scilab/modules/xcos/src/java/org/scilab/modules/xcos/XcosViewListener.java

index bf9eff1..2c35c68 100644 (file)
@@ -16,6 +16,7 @@
 package org.scilab.modules.xcos;
 
 import org.scilab.modules.graph.utils.StyleMap;
+import org.scilab.modules.xcos.graph.model.ScicosObjectOwner;
 
 /**
  * Update the source block when the interface function change.
@@ -32,8 +33,8 @@ public final class UpdateStyleFromInterfunctionAdapter extends XcosViewListener
      * and not newStyle="fillColor=red;DSUPER"
      */
     @Override
-    public void propertyUpdated(long uid, Kind kind, ObjectProperties property, UpdateStatus status) {
-        if (status != UpdateStatus.SUCCESS || kind != Kind.BLOCK) {
+    public void propertyUpdated(ScicosObjectOwner owner, ObjectProperties property, UpdateStatus status) {
+        if (status != UpdateStatus.SUCCESS || owner.getKind() != Kind.BLOCK) {
             return;
         }
 
@@ -45,15 +46,15 @@ public final class UpdateStyleFromInterfunctionAdapter extends XcosViewListener
         JavaController controller = new JavaController();
 
         String[] interfaceFunction = new String[1];
-        controller.getObjectProperty(uid, kind, ObjectProperties.INTERFACE_FUNCTION, interfaceFunction);
+        controller.getObjectProperty(owner.getUID(), owner.getKind(), ObjectProperties.INTERFACE_FUNCTION, interfaceFunction);
 
         String[] style = new String[1];
-        controller.getObjectProperty(uid, kind, ObjectProperties.STYLE, style);
+        controller.getObjectProperty(owner.getUID(), owner.getKind(), ObjectProperties.STYLE, style);
 
 
         final StyleMap styleMap = new StyleMap(interfaceFunction[0]);
         styleMap.putAll(style[0]);
 
-        controller.setObjectProperty(uid, kind, ObjectProperties.STYLE, styleMap.toString());
+        controller.setObjectProperty(owner.getUID(), owner.getKind(), ObjectProperties.STYLE, styleMap.toString());
     }
 }
index d596938..4828250 100644 (file)
@@ -1,6 +1,7 @@
 /*
  * Scilab ( http://www.scilab.org/ ) - This file is part of Scilab
  * Copyright (C) 2015 - Scilab Enterprises - Clement DAVID
+ * Copyright (C) 2018 - ESI Group - Clement DAVID
  *
  * Copyright (C) 2012 - 2016 - Scilab Enterprises
  *
@@ -21,6 +22,7 @@ import java.util.HashMap;
 import java.util.List;
 
 import javax.swing.SwingUtilities;
+import org.scilab.modules.xcos.graph.model.ScicosObjectOwner;
 
 /**
  * Generic view to dispatch common GUI update to the right JGraphX component
@@ -63,32 +65,24 @@ public final class XcosView extends View {
     /*
      * Implement the MVC View interface by dispatch-ing to the listeners
      */
-
+    
     @Override
     public final void objectCreated(long uid, Kind kind) {
         List<Entry> listeners = registeredListeners.get(kind);
         if (listeners == null) {
             return;
         }
-
+        
         for (Entry e : listeners) {
             if (e.onCallerThread) {
                 e.listener.objectCreated(uid, kind);
             } else {
-                // reference the object on the caller thread to avoid premature deletion
-                new JavaController().referenceObject(uid);
-
                 SwingUtilities.invokeLater(new Runnable() {
-                    @Override
-                    public void run() {
-                        try {
+                        @Override
+                        public void run() {
                             e.listener.objectCreated(uid, kind);
-                        } finally {
-                            // unreference the object on the EDT
-                            new JavaController().deleteObject(uid);
                         }
-                    }
-                });
+                    });
             }
         }
     }
@@ -160,34 +154,42 @@ public final class XcosView extends View {
     public void objectCloned(long uid, long cloned, Kind kind) {
     }
 
+    // ensure ownership through different threads (Scilab, EDT, GC)
+    private static final class PropertyUpdatedRunnable implements Runnable {
+        private final Entry e;
+        private final ScicosObjectOwner owner;
+        private final ObjectProperties property;
+        private final UpdateStatus status;
+        
+        public PropertyUpdatedRunnable(Entry e, ScicosObjectOwner owner, ObjectProperties property, UpdateStatus status) {
+            this.e = e;
+            this.owner = owner;
+            this.property = property;
+            this.status = status;
+        }
+
+        @Override
+        public void run() {
+            e.listener.propertyUpdated(owner, property, status);
+        }
+    }
+    
     @Override
     public final void propertyUpdated(long uid, Kind kind, ObjectProperties property, UpdateStatus status) {
         List<Entry> listeners = registeredListeners.get(kind);
         if (listeners == null) {
             return;
         }
-
+        
+        final ScicosObjectOwner owner = new ScicosObjectOwner(uid, kind);
         for (Entry e : listeners) {
             if (e.onCallerThread) {
                 if (e.listenedProperties.contains(property)) {
-                    e.listener.propertyUpdated(uid, kind, property, status);
+                    e.listener.propertyUpdated(owner, property, status);
                 }
             } else {
                 if (e.listenedProperties.contains(property)) {
-                    // reference the object on the caller thread to avoid premature deletion
-                    new JavaController().referenceObject(uid);
-
-                    SwingUtilities.invokeLater(new Runnable() {
-                        @Override
-                        public void run() {
-                            try {
-                                e.listener.propertyUpdated(uid, kind, property, status);
-                            } finally {
-                                // unreference the object on the EDT
-                                new JavaController().deleteObject(uid);
-                            }
-                        }
-                    });
+                    SwingUtilities.invokeLater(new PropertyUpdatedRunnable(e, owner, property, status));
                 }
             }
         }
index e28476a..665eadf 100644 (file)
@@ -14,6 +14,8 @@
  */
 package org.scilab.modules.xcos;
 
+import org.scilab.modules.xcos.graph.model.ScicosObjectOwner;
+
 /**
  * Dummy abstract implementation of an XcosView listener.
  */
@@ -22,5 +24,5 @@ public abstract class XcosViewListener {
     public void objectDeleted(long uid, Kind kind) {};
     public void objectReferenced(long uid, Kind kind, long refCount) {};
     public void objectUnreferenced(long uid, Kind kind, long refCount) {};
-    public void propertyUpdated(long uid, Kind kind, ObjectProperties property, UpdateStatus status) {};
+    public void propertyUpdated(ScicosObjectOwner owner, ObjectProperties property, UpdateStatus status) {};
 }