Skip to content

Fixed recursive dependency in clr module initialization #1602

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/embed_tests/pyimport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

25 changes: 16 additions & 9 deletions src/runtime/importhook.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Concurrent;
using System.Diagnostics;

namespace Python.Runtime
{
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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>
Expand All @@ -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();
Expand All @@ -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)
Expand Down
7 changes: 4 additions & 3 deletions src/runtime/pythonengine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -257,6 +256,8 @@ public static void Initialize(IEnumerable<string> args, bool setSysArgv = true,
{
locals.Dispose();
}

ImportHook.UpdateCLRModuleDict();
}

static BorrowedReference DefineModule(string name)
Expand Down