Fixes to thread safety problems in SCL compiler

[simantics/platform.git] / bundles / org.simantics.scl.compiler / src / org / simantics / scl / compiler / module / repository / ModuleRepository.java

diff --git a/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/module/repository/ModuleRepository.java b/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/module/repository/ModuleRepository.java
index 0f0b8542049a53c7f8eea2667ba15c40789f0f7b..8a86c5ccce36ecef59326e7abcd32a558d7e27ea 100644 (file)
--- a/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/module/repository/ModuleRepository.java
+++ b/bundles/org.simantics.scl.compiler/src/org/simantics/scl/compiler/module/repository/ModuleRepository.java
@@ -70,10 +70,11 @@ public class ModuleRepository {
     private static void finishModuleCompilation(String moduleName) {
         PENDING_MODULES.get().remove(moduleName);
     }
-    
+            
     private class ModuleEntry extends UpdateListener implements Observable {
         final String moduleName;
-        THashSet<UpdateListener> listeners = new THashSet<UpdateListener>();
+        THashSet<UpdateListener> listeners = new THashSet<UpdateListener>(); // listeners == null is used as a marker that this entry is disposed
+                                                                             // should be handled only inside synchronized code
         
         ModuleSource source;
         Failable<Module> compilationResult;
@@ -98,33 +99,38 @@ public class ModuleRepository {
         
         @Override
         public void notifyAboutUpdate() {
+            // There is a chance that another observable calls notifyAboutUpdate() before stopListening has been completed,
+            // but notifyAboutUpdate(ArrayList<UpdateListener>) lets only one thread to do the notification of dependencies
+            // by clearing listeners field.
+            stopListening();
             ArrayList<UpdateListener> externalListeners = new ArrayList<UpdateListener>();
             notifyAboutUpdate(externalListeners);
             for(UpdateListener listener : externalListeners)
                 listener.notifyAboutUpdate();
         }
 
-        synchronized void notifyAboutUpdate(ArrayList<UpdateListener> externalListeners) {
-            stopListening();
-            if (listeners == null)
-                return;
+        void notifyAboutUpdate(ArrayList<UpdateListener> externalListeners) {
+            THashSet<UpdateListener> listenersCopy;
+            synchronized(this) {
+                listenersCopy = listeners;
+                if (listenersCopy == null)
+                    return;
+                listeners = null;
+            }
             if(moduleCache.get(moduleName) == this) {
                 moduleCache.remove(moduleName);
                 if(SCLCompilerConfiguration.TRACE_MODULE_UPDATE) {
                     System.out.println("Invalidate " + moduleName);
-                    for(UpdateListener l : listeners)
+                    for(UpdateListener l : listenersCopy)
                         System.out.println("    " + l);
                 }
-                THashSet<UpdateListener> listenersCopy = listeners;
-                listeners = null;
-                for(UpdateListener l : listenersCopy)
-                    l.stopListening();
                 for(UpdateListener l : listenersCopy)
-                    if(l instanceof ModuleEntry)
+                    if(!l.stopListening())
+                        ;
+                    else if(l instanceof ModuleEntry)
                         ((ModuleEntry)l).notifyAboutUpdate(externalListeners);
-                    else {
+                    else
                         externalListeners.add(l);
-                    }
             }
         }
 
@@ -195,8 +201,6 @@ public class ModuleRepository {
         }
 
         public synchronized void dispose() {
-            if (listeners != null)
-                listeners.clear();
             listeners = null;
             stopListening();
             source = null;
@@ -490,18 +494,24 @@ public class ModuleRepository {
         return documentation;
     }
     
+    /**
+     * Flush clears module repository cache completely. It should not be called in 
+     * normal operation, but may be useful during testing for clearing repositories
+     * that are statically defined.
+     */
     public void flush() {
         if (parentRepository != null)
             parentRepository.flush();
-        if (moduleCache != null) {
-            for (ModuleEntry entry : moduleCache.values()) {
+        if (moduleCache != null)
+            for (ModuleEntry entry : moduleCache.values())
                 entry.dispose();
-            }
-            moduleCache.clear();
-        }
         moduleCache = null;
     }
 
+    /**
+     * Gets the map of all modules that have been currently compiled successfully.
+     * Not that the method does not return all possible modules in the source repository.
+     */
     public Map<String, Module> getModules() {
         Map<String, Module> result = new HashMap<>(moduleCache.size()); 
         for (Map.Entry<String, ModuleEntry> entry : moduleCache.entrySet()) {