Skip to content

Import hook clean-up #350

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.

Sign up for GitHub

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 2 commits into from
Feb 2, 2017
Merged

Import hook clean-up #350

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
26ce7d8
Alias root.pyHandle to py_clr_module
vmuriart Feb 1, 2017
0e74f86
Syntax cleanup
vmuriart Feb 1, 2017
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
33 changes: 11 additions & 22 deletions src/runtime/importhook.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ internal class ImportHook
static IntPtr py_import;
static CLRModule root;
static MethodWrapper hook;
static IntPtr py_clr_module;

#if PYTHON3
static IntPtr py_clr_module;
static IntPtr module_def = IntPtr.Zero;

internal static void InitializeModuleDef()
Expand All @@ -36,11 +36,10 @@ internal static void Initialize()
IntPtr dict = Runtime.PyImport_GetModuleDict();
#if PYTHON3
IntPtr mod = Runtime.PyImport_ImportModule("builtins");
py_import = Runtime.PyObject_GetAttrString(mod, "__import__");
#elif PYTHON2
IntPtr mod = Runtime.PyDict_GetItemString(dict, "__builtin__");
py_import = Runtime.PyObject_GetAttrString(mod, "__import__");
#endif
py_import = Runtime.PyObject_GetAttrString(mod, "__import__");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could instead use PyEval_GetBuiltins() on both Py2 and 3.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saw this comment after I merged it in 😞

hook = new MethodWrapper(typeof(ImportHook), "__import__", "TernaryFunc");
Runtime.PyObject_SetAttrString(mod, "__import__", hook.ptr);
Runtime.XDecref(hook.ptr);
Expand All @@ -58,13 +57,12 @@ internal static void Initialize()
clr_dict = (IntPtr)Marshal.PtrToStructure(clr_dict, typeof(IntPtr));

Runtime.PyDict_Update(mod_dict, clr_dict);
Runtime.PyDict_SetItemString(dict, "CLR", py_clr_module);
Runtime.PyDict_SetItemString(dict, "clr", py_clr_module);
#elif PYTHON2
Runtime.XIncref(root.pyHandle); // we are using the module two times
Runtime.PyDict_SetItemString(dict, "CLR", root.pyHandle);
Runtime.PyDict_SetItemString(dict, "clr", root.pyHandle);
py_clr_module = root.pyHandle; // Alias handle for PY2/PY3
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@filmor Since you just worked on this, any reason this wouldn't work or is a bad idea?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, this should work fine, IMO.

#endif
Runtime.PyDict_SetItemString(dict, "CLR", py_clr_module);
Runtime.PyDict_SetItemString(dict, "clr", py_clr_module);
}


Expand All @@ -73,13 +71,9 @@ internal static void Initialize()
/// </summary>
internal static void Shutdown()
{
if (0 != Runtime.Py_IsInitialized())
if (Runtime.Py_IsInitialized() != 0)
{
#if PYTHON3
Runtime.XDecref(py_clr_module);
#elif PYTHON2
Runtime.XDecref(root.pyHandle);
#endif
Runtime.XDecref(root.pyHandle);
Runtime.XDecref(py_import);
}
Expand Down Expand Up @@ -117,11 +111,11 @@ public static IntPtr GetCLRModule(IntPtr? fromList = null)
continue;

string s = item.AsManagedObject(typeof(string)) as string;
if (null == s)
if (s == null)
continue;

ManagedType attr = root.GetAttribute(s, true);
if (null == attr)
if (attr == null)
continue;

Runtime.XIncref(attr.pyHandle);
Expand All @@ -134,13 +128,9 @@ public static IntPtr GetCLRModule(IntPtr? fromList = null)
}
}
}

#endif
Runtime.XIncref(py_clr_module);
return py_clr_module;
#elif PYTHON2
Runtime.XIncref(root.pyHandle);
return root.pyHandle;
#endif
}

/// <summary>
Expand Down Expand Up @@ -200,7 +190,7 @@ public static IntPtr __import__(IntPtr self, IntPtr args, IntPtr kw)
}
if (mod_name == "CLR")
{
Exceptions.deprecation("The CLR module is deprecated. " + "Please use 'clr'.");
Exceptions.deprecation("The CLR module is deprecated. Please use 'clr'.");
IntPtr clr_module = GetCLRModule(fromList);
if (clr_module != IntPtr.Zero)
{
Expand Down Expand Up @@ -315,9 +305,8 @@ public static IntPtr __import__(IntPtr self, IntPtr args, IntPtr kw)
ModuleObject tail = root;
root.InitializePreload();

for (int i = 0; i < names.Length; i++)
foreach (string name in names)
{
string name = names[i];
ManagedType mt = tail.GetAttribute(name, true);
if (!(mt is ModuleObject))
{
Expand Down