Add Namespaces to the import hook only through AddReference

BadSingleton · BadSingleton · commit ff170e99b78e · 2021-06-02T16:07:27.000-04:00
Don't piggyback on AssemblyManager's AssemblyLoadHandler method because
it may be called from other threads, leading to deadlocks if it is
called while Python code is executing
diff --git a/src/runtime/assemblymanager.cs b/src/runtime/assemblymanager.cs
@@ -37,10 +37,6 @@ internal class AssemblyManager
         // modified from event handlers below, potentially triggered from different .NET threads
         private static ConcurrentQueue<Assembly> assemblies;
         internal static List<string> pypath;
-        
-        // Triggered when a new namespace is added to the namespaces dictionary
-        // public static event Action<string> namespaceAdded;
-
         private AssemblyManager()
         {
         }
@@ -287,17 +283,6 @@ internal static void ScanAssembly(Assembly assembly)
                 if (ns != null)
                 {
                     namespaces[ns].TryAdd(assembly, string.Empty);
-                    // try
-                    // {
-                    //     namespaceAdded?.Invoke(ns);
-                    // }
-                    // catch (Exception e)
-                    // {
-                    //     // For some reason, exceptions happening here does... nothing.
-                    //     // Even System.AccessViolationExceptions gets ignored.
-                    //     Console.WriteLine($"Namespace added callback failed with: {e}");
-                    //     throw;
-                    // }
                 }
 
                 if (ns != null && t.IsGenericTypeDefinition)
diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs
@@ -176,30 +176,21 @@ static void SetupNamespaceTracking()
                 newset.Dispose();
             }
 
-            // AssemblyManager.namespaceAdded += OnNamespaceAdded;
-            // PythonEngine.AddShutdownHandler(() => AssemblyManager.namespaceAdded -= OnNamespaceAdded);
         }
 
         /// <summary>
-        /// Removes the set of available namespaces from the clr module and 
-        /// removes the callback on the OnNamespaceAdded event.
+        /// Removes the set of available namespaces from the clr module.
         /// </summary>
         static void TeardownNameSpaceTracking()
         {
-            // AssemblyManager.namespaceAdded -= OnNamespaceAdded;
             // If the C# runtime isn't loaded, then there are no namespaces available
             Runtime.PyDict_SetItemString(root.dict, availableNsKey, Runtime.PyNone);
         }
 
-        public static void OnNamespaceAdded(string name)
+        public static void AddNamespace(string name)
         {
-            Console.WriteLine(System.Environment.StackTrace);
-            Console.WriteLine("OnNamespaceAdded: acquiring");
-            Console.Out.Flush();
             using (Py.GIL())
             {
-                Console.WriteLine("OnNamespaceAdded: acquired");
-                Console.Out.Flush();
                 var pyNs = Runtime.PyString_FromString(name);
                 try
                 {
@@ -217,8 +208,6 @@ public static void OnNamespaceAdded(string name)
                     Runtime.XDecref(pyNs);
                 }
             }
-            Console.WriteLine("OnNamespaceAdded: released");
-            Console.Out.Flush();
         }
 
 
diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs
@@ -509,6 +509,7 @@ public static bool SuppressOverloads
         public static Assembly AddReference(string name)
         {
             AssemblyManager.UpdatePath();
+            var origNs = AssemblyManager.GetNamespaces();
             Assembly assembly = null;
             assembly = AssemblyManager.FindLoadedAssembly(name);
             if (assembly == null)
@@ -530,9 +531,12 @@ public static Assembly AddReference(string name)
             // Classes that are not in a namespace needs an extra nudge to be found.
             ImportHook.UpdateCLRModuleDict();
 
-            // Heavyhanded but otherwise we'd need a "addedSinceLastCall".
-            foreach(var ns in AssemblyManager.GetNamespaces()){
-                ImportHook.OnNamespaceAdded(ns);
+            // A bit heavyhanded, but we can't use the AssemblyManager's AssemblyLoadHandler
+            // method because it may be called from other threads, leading to deadlocks
+            // if it is called while Python code is executing.
+            var currNs = AssemblyManager.GetNamespaces().Except(origNs);
+            foreach(var ns in currNs){
+                ImportHook.AddNamespace(ns);
             }
             return assembly;
         }

Original file line number	Diff line number	Diff line change
`@@ -176,30 +176,21 @@ static void SetupNamespaceTracking()`
`176`	`176`	`newset.Dispose();`
`177`	`177`	`}`
`178`	`178`
`179`		`- // AssemblyManager.namespaceAdded += OnNamespaceAdded;`
`180`		`- // PythonEngine.AddShutdownHandler(() => AssemblyManager.namespaceAdded -= OnNamespaceAdded);`
`181`	`179`	`}`
`182`	`180`
`183`	`181`	`/// <summary>`
`184`		`- /// Removes the set of available namespaces from the clr module and`
`185`		`- /// removes the callback on the OnNamespaceAdded event.`
	`182`	`+ /// Removes the set of available namespaces from the clr module.`
`186`	`183`	`/// </summary>`
`187`	`184`	`static void TeardownNameSpaceTracking()`
`188`	`185`	`{`
`189`		`- // AssemblyManager.namespaceAdded -= OnNamespaceAdded;`
`190`	`186`	`// If the C# runtime isn't loaded, then there are no namespaces available`
`191`	`187`	`Runtime.PyDict_SetItemString(root.dict, availableNsKey, Runtime.PyNone);`
`192`	`188`	`}`
`193`	`189`
`194`		`- public static void OnNamespaceAdded(string name)`
	`190`	`+ public static void AddNamespace(string name)`
`195`	`191`	`{`
`196`		`- Console.WriteLine(System.Environment.StackTrace);`
`197`		`- Console.WriteLine("OnNamespaceAdded: acquiring");`
`198`		`- Console.Out.Flush();`
`199`	`192`	`using (Py.GIL())`
`200`	`193`	`{`
`201`		`- Console.WriteLine("OnNamespaceAdded: acquired");`
`202`		`- Console.Out.Flush();`
`203`	`194`	`var pyNs = Runtime.PyString_FromString(name);`
`204`	`195`	`try`
`205`	`196`	`{`
`@@ -217,8 +208,6 @@ public static void OnNamespaceAdded(string name)`
`217`	`208`	`Runtime.XDecref(pyNs);`
`218`	`209`	`}`
`219`	`210`	`}`
`220`		`- Console.WriteLine("OnNamespaceAdded: released");`
`221`		`- Console.Out.Flush();`
`222`	`211`	`}`
`223`	`212`
`224`	`213`