fixed recursive dependency in clr module initialization when there's a public class that implements IEnumerable in global namespace

lostmsu · lostmsu · commit 3f335db25d11 · 2021-10-16T16:44:50.000-07:00
#1601 the fix is to delay updating clr module dict with contents of .NET namespaces until after our internal modules are loaded
diff --git a/src/embed_tests/pyimport.cs b/src/embed_tests/pyimport.cs
@@ -112,3 +112,14 @@ import clr
         }
     }
 }
+
+// regression for https://github.com/pythonnet/pythonnet/issues/1601
+// initialize fails if a class derived from IEnumerable is in global namespace
+public class PublicEnumerator : System.Collections.IEnumerable
+{
+    public System.Collections.IEnumerator GetEnumerator()
+    {
+        return null;
+    }
+}
+
diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Concurrent;
+using System.Diagnostics;
 
 namespace Python.Runtime
 {
@@ -10,7 +11,14 @@ internal static class ImportHook
     {
         private static CLRModule root;
         private static IntPtr py_clr_module;
-        static BorrowedReference ClrModuleReference => new BorrowedReference(py_clr_module);
+        internal static BorrowedReference ClrModuleReference
+        {
+            get
+            {
+                Debug.Assert(py_clr_module != IntPtr.Zero);
+                return new BorrowedReference(py_clr_module);
+            }
+        }
 
         private const string LoaderCode = @"
 import importlib.abc
@@ -43,7 +51,7 @@ def find_spec(klass, fullname, paths=None, target=None):
             return importlib.machinery.ModuleSpec(fullname, DotNetLoader(), is_package=True)
         return None
             ";
-        const string availableNsKey = "_available_namespaces";
+        const string _available_namespaces = "_available_namespaces";
 
         /// <summary>
         /// Initialization performed on startup of the Python runtime.
@@ -154,12 +162,11 @@ static void SetupNamespaceTracking()
                 {
                     throw PythonException.ThrowLastAsClrException();
                 }
-                if (Runtime.PyDict_SetItemString(root.DictRef, availableNsKey, newset) != 0)
-                {
-                    throw PythonException.ThrowLastAsClrException();
-                }
             }
-
+            if (Runtime.PyDict_SetItemString(root.DictRef, _available_namespaces, newset) != 0)
+            {
+                throw PythonException.ThrowLastAsClrException();
+            }
         }
 
         /// <summary>
@@ -168,7 +175,7 @@ static void SetupNamespaceTracking()
         static void TeardownNameSpaceTracking()
         {
             // If the C# runtime isn't loaded, then there are no namespaces available
-            Runtime.PyDict_SetItemString(root.dict, availableNsKey, Runtime.PyNone);
+            Runtime.PyDict_SetItemString(root.dict, _available_namespaces, Runtime.PyNone);
         }
 
         static readonly ConcurrentQueue<string> addPending = new();
@@ -190,7 +197,7 @@ internal static void AddNamespaceWithGIL(string name)
             var pyNs = Runtime.PyString_FromString(name);
             try
             {
-                var nsSet = Runtime.PyDict_GetItemString(root.DictRef, availableNsKey);
+                var nsSet = Runtime.PyDict_GetItemString(root.DictRef, _available_namespaces);
                 if (!(nsSet.IsNull  || nsSet.DangerousGetAddress() == Runtime.PyNone))
                 {
                     if (Runtime.PySet_Add(nsSet, new BorrowedReference(pyNs)) != 0)
diff --git a/src/runtime/pythonengine.cs b/src/runtime/pythonengine.cs
@@ -220,8 +220,7 @@ public static void Initialize(IEnumerable<string> args, bool setSysArgv = true,
             }
 
             // Load the clr.py resource into the clr module
-            NewReference clr = Python.Runtime.ImportHook.GetCLRModule();
-            BorrowedReference clr_dict = Runtime.PyModule_GetDict(clr);
+            BorrowedReference clr_dict = Runtime.PyModule_GetDict(ImportHook.ClrModuleReference);
 
             var locals = new PyDict();
             try
@@ -236,7 +235,7 @@ public static void Initialize(IEnumerable<string> args, bool setSysArgv = true,
 
                 LoadSubmodule(module_globals, "clr.interop", "interop.py");
 
-                    LoadMixins(module_globals);
+                LoadMixins(module_globals);
 
                 // add the imported module to the clr module, and copy the API functions
                 // and decorators into the main clr module.
@@ -257,6 +256,8 @@ public static void Initialize(IEnumerable<string> args, bool setSysArgv = true,
             {
                 locals.Dispose();
             }
+
+            ImportHook.UpdateCLRModuleDict();
         }
 
         static BorrowedReference DefineModule(string name)

Original file line number	Diff line number	Diff line change
`@@ -112,3 +112,14 @@ import clr`
`112`	`112`	`}`
`113`	`113`	`}`
`114`	`114`	`}`
	`115`	`+`
	`116`	`+// regression for https://github.com/pythonnet/pythonnet/issues/1601`
	`117`	`+// initialize fails if a class derived from IEnumerable is in global namespace`
	`118`	`+public class PublicEnumerator : System.Collections.IEnumerable`
	`119`	`+{`
	`120`	`+ public System.Collections.IEnumerator GetEnumerator()`
	`121`	`+ {`
	`122`	`+ return null;`
	`123`	`+ }`
	`124`	`+}`
	`125`	`+`
Original file line number	Diff line number	Diff line change
`@@ -220,8 +220,7 @@ public static void Initialize(IEnumerable<string> args, bool setSysArgv = true,`
`220`	`220`	`}`
`221`	`221`
`222`	`222`	`// Load the clr.py resource into the clr module`
`223`		`- NewReference clr = Python.Runtime.ImportHook.GetCLRModule();`
`224`		`- BorrowedReference clr_dict = Runtime.PyModule_GetDict(clr);`
	`223`	`+ BorrowedReference clr_dict = Runtime.PyModule_GetDict(ImportHook.ClrModuleReference);`
`225`	`224`
`226`	`225`	`var locals = new PyDict();`
`227`	`226`	`try`
`@@ -236,7 +235,7 @@ public static void Initialize(IEnumerable<string> args, bool setSysArgv = true,`
`236`	`235`
`237`	`236`	`LoadSubmodule(module_globals, "clr.interop", "interop.py");`
`238`	`237`
`239`		`- LoadMixins(module_globals);`
	`238`	`+ LoadMixins(module_globals);`
`240`	`239`
`241`	`240`	`// add the imported module to the clr module, and copy the API functions`
`242`	`241`	`// and decorators into the main clr module.`
`@@ -257,6 +256,8 @@ public static void Initialize(IEnumerable<string> args, bool setSysArgv = true,`
`257`	`256`	`{`
`258`	`257`	`locals.Dispose();`
`259`	`258`	`}`
	`259`	`+`
	`260`	`+ ImportHook.UpdateCLRModuleDict();`
`260`	`261`	`}`
`261`	`262`
`262`	`263`	`static BorrowedReference DefineModule(string name)`