Skip to content

Commit 645687e

Browse files
committed
do not eat all exceptions when trying to add CLR reference, provide more info when CLR assemblies are failed to be loaded
1. When trying to implicitly load assemblies, and that fails NOT because an assembly is missing, but because loading failed for some reason, emit Python warning. 2. When trying to import a module in our import hook, if the module name is an assembly name, and we fail to load it, and Python also fails to find a module with the same name, add the exceptions we got during the attempt to load it into __cause__ of the final ImportError BREAKING: clr.AddReference will now throw exceptions besides FileNotFoundException. Additional: a few uses of BorrowedReference This addresses #261 It is an alternative to #298
1 parent 9fd877e commit 645687e

File tree

9 files changed

+99
-71
lines changed

9 files changed

+99
-71
lines changed

src/embed_tests/pyimport.cs

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ public void SetUp()
3838
string testPath = Path.Combine(TestContext.CurrentContext.TestDirectory, s);
3939

4040
IntPtr str = Runtime.Runtime.PyString_FromString(testPath);
41-
IntPtr path = Runtime.Runtime.PySys_GetObject("path");
42-
Runtime.Runtime.PyList_Append(new BorrowedReference(path), str);
41+
BorrowedReference path = Runtime.Runtime.PySys_GetObject("path");
42+
Runtime.Runtime.PyList_Append(path, str);
4343
}
4444

4545
[TearDown]
@@ -83,5 +83,28 @@ public void TestCastGlobalVar()
8383
Assert.AreEqual("2", foo.FOO.ToString());
8484
Assert.AreEqual("2", foo.test_foo().ToString());
8585
}
86+
87+
[Test]
88+
public void BadAssembly()
89+
{
90+
string path;
91+
if (Python.Runtime.Runtime.IsWindows)
92+
{
93+
path = @"C:\Windows\System32\kernel32.dll";
94+
}
95+
else
96+
{
97+
Assert.Pass("TODO: add bad assembly location for other platforms");
98+
return;
99+
}
100+
101+
string code = $@"
102+
import clr
103+
clr.AddReference('{path}')
104+
";
105+
106+
var error = Assert.Throws<PythonException>(() => PythonEngine.Exec(code));
107+
Assert.AreEqual(nameof(FileLoadException), error.PythonTypeName);
108+
}
86109
}
87110
}

src/runtime/assemblymanager.cs

Lines changed: 25 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ private static Assembly ResolveHandler(object ob, ResolveEventArgs args)
137137
/// </summary>
138138
internal static void UpdatePath()
139139
{
140-
IntPtr list = Runtime.PySys_GetObject("path");
140+
BorrowedReference list = Runtime.PySys_GetObject("path");
141141
var count = Runtime.PyList_Size(list);
142142
if (count != pypath.Count)
143143
{
@@ -199,19 +199,14 @@ public static string FindAssembly(string name)
199199
/// </summary>
200200
public static Assembly LoadAssembly(string name)
201201
{
202-
Assembly assembly = null;
203202
try
204203
{
205-
assembly = Assembly.Load(name);
204+
return Assembly.Load(name);
206205
}
207-
catch (Exception)
206+
catch (FileNotFoundException)
208207
{
209-
//if (!(e is System.IO.FileNotFoundException))
210-
//{
211-
// throw;
212-
//}
208+
return null;
213209
}
214-
return assembly;
215210
}
216211

217212

@@ -221,18 +216,8 @@ public static Assembly LoadAssembly(string name)
221216
public static Assembly LoadAssemblyPath(string name)
222217
{
223218
string path = FindAssembly(name);
224-
Assembly assembly = null;
225-
if (path != null)
226-
{
227-
try
228-
{
229-
assembly = Assembly.LoadFrom(path);
230-
}
231-
catch (Exception)
232-
{
233-
}
234-
}
235-
return assembly;
219+
if (path == null) return null;
220+
return Assembly.LoadFrom(path);
236221
}
237222

238223
/// <summary>
@@ -242,25 +227,18 @@ public static Assembly LoadAssemblyPath(string name)
242227
/// <returns></returns>
243228
public static Assembly LoadAssemblyFullPath(string name)
244229
{
245-
Assembly assembly = null;
246230
if (Path.IsPathRooted(name))
247231
{
248-
if (!Path.HasExtension(name))
232+
if (!Path.HasExtension(name) && Runtime.InteropVersion < new Version(3, 0))
249233
{
250234
name = name + ".dll";
251235
}
252236
if (File.Exists(name))
253237
{
254-
try
255-
{
256-
assembly = Assembly.LoadFrom(name);
257-
}
258-
catch (Exception)
259-
{
260-
}
238+
return Assembly.LoadFrom(name);
261239
}
262240
}
263-
return assembly;
241+
return null;
264242
}
265243

266244
/// <summary>
@@ -291,7 +269,7 @@ public static Assembly FindLoadedAssembly(string name)
291269
/// actually loads an assembly.
292270
/// Call ONLY for namespaces that HAVE NOT been cached yet.
293271
/// </remarks>
294-
public static bool LoadImplicit(string name, bool warn = true)
272+
public static bool LoadImplicit(string name, Action<Exception> assemblyLoadErrorHandler, bool warn = true)
295273
{
296274
string[] names = name.Split('.');
297275
var loaded = false;
@@ -308,14 +286,23 @@ public static bool LoadImplicit(string name, bool warn = true)
308286
assembliesSet = new HashSet<Assembly>(AppDomain.CurrentDomain.GetAssemblies());
309287
}
310288
Assembly a = FindLoadedAssembly(s);
311-
if (a == null)
312-
{
313-
a = LoadAssemblyPath(s);
314-
}
315-
if (a == null)
289+
try
316290
{
317-
a = LoadAssembly(s);
291+
if (a == null)
292+
{
293+
a = LoadAssemblyPath(s);
294+
}
295+
296+
if (a == null)
297+
{
298+
a = LoadAssembly(s);
299+
}
318300
}
301+
catch (FileLoadException e) { assemblyLoadErrorHandler(e); }
302+
catch (BadImageFormatException e) { assemblyLoadErrorHandler(e); }
303+
catch (System.Security.SecurityException e) { assemblyLoadErrorHandler(e); }
304+
catch (PathTooLongException e) { assemblyLoadErrorHandler(e); }
305+
319306
if (a != null && !assembliesSet.Contains(a))
320307
{
321308
loaded = true;

src/runtime/exceptions.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,9 +256,9 @@ public static void SetError(IntPtr ob, string value)
256256
/// Sets the current Python exception given a Python object.
257257
/// This is a wrapper for the Python PyErr_SetObject call.
258258
/// </remarks>
259-
public static void SetError(IntPtr ob, IntPtr value)
259+
public static void SetError(IntPtr type, IntPtr exceptionObject)
260260
{
261-
Runtime.PyErr_SetObject(ob, value);
261+
Runtime.PyErr_SetObject(new BorrowedReference(type), new BorrowedReference(exceptionObject));
262262
}
263263

264264
/// <summary>
@@ -288,7 +288,7 @@ public static void SetError(Exception e)
288288

289289
IntPtr op = CLRObject.GetInstHandle(e);
290290
IntPtr etype = Runtime.PyObject_GetAttrString(op, "__class__");
291-
Runtime.PyErr_SetObject(etype, op);
291+
Runtime.PyErr_SetObject(new BorrowedReference(etype), new BorrowedReference(op));
292292
Runtime.XDecref(etype);
293293
Runtime.XDecref(op);
294294
}

src/runtime/importhook.cs

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Collections.Generic;
23
using System.Runtime.InteropServices;
34

45
namespace Python.Runtime
@@ -238,22 +239,12 @@ public static IntPtr __import__(IntPtr self, IntPtr args, IntPtr kw)
238239
// Check these BEFORE the built-in import runs; may as well
239240
// do the Incref()ed return here, since we've already found
240241
// the module.
241-
if (mod_name == "clr")
242+
if (mod_name == "clr" || mod_name == "CLR")
242243
{
243-
IntPtr clr_module = GetCLRModule(fromList);
244-
if (clr_module != IntPtr.Zero)
244+
if (mod_name == "CLR")
245245
{
246-
IntPtr sys_modules = Runtime.PyImport_GetModuleDict();
247-
if (sys_modules != IntPtr.Zero)
248-
{
249-
Runtime.PyDict_SetItemString(sys_modules, "clr", clr_module);
250-
}
246+
Exceptions.deprecation("The CLR module is deprecated. Please use 'clr'.");
251247
}
252-
return clr_module;
253-
}
254-
if (mod_name == "CLR")
255-
{
256-
Exceptions.deprecation("The CLR module is deprecated. Please use 'clr'.");
257248
IntPtr clr_module = GetCLRModule(fromList);
258249
if (clr_module != IntPtr.Zero)
259250
{
@@ -265,9 +256,10 @@ public static IntPtr __import__(IntPtr self, IntPtr args, IntPtr kw)
265256
}
266257
return clr_module;
267258
}
259+
268260
string realname = mod_name;
269261
string clr_prefix = null;
270-
if (mod_name.StartsWith("CLR."))
262+
if (mod_name.StartsWith("CLR.") && Runtime.InteropVersion < new Version(3,0))
271263
{
272264
clr_prefix = "CLR."; // prepend when adding the module to sys.modules
273265
realname = mod_name.Substring(4);
@@ -328,11 +320,22 @@ public static IntPtr __import__(IntPtr self, IntPtr args, IntPtr kw)
328320
AssemblyManager.UpdatePath();
329321
if (!AssemblyManager.IsValidNamespace(realname))
330322
{
331-
if (!AssemblyManager.LoadImplicit(realname))
323+
var loadExceptions = new List<Exception>();
324+
if (!AssemblyManager.LoadImplicit(realname, assemblyLoadErrorHandler: loadExceptions.Add))
332325
{
333326
// May be called when a module being imported imports a module.
334327
// In particular, I've seen decimal import copy import org.python.core
335-
return Runtime.PyObject_Call(py_import, args, kw);
328+
IntPtr importResult = Runtime.PyObject_Call(py_import, args, kw);
329+
// TODO: use ModuleNotFoundError in Python 3.6+
330+
if (importResult == IntPtr.Zero && loadExceptions.Count > 0
331+
&& Exceptions.ExceptionMatches(Exceptions.ImportError))
332+
{
333+
loadExceptions.Add(new PythonException());
334+
var importError = new PyObject(new BorrowedReference(Exceptions.ImportError));
335+
importError.SetAttr("__cause__", new AggregateException(loadExceptions).ToPython());
336+
Runtime.PyErr_SetObject(new BorrowedReference(Exceptions.ImportError), importError.Reference);
337+
}
338+
return importResult;
336339
}
337340
}
338341

src/runtime/methodbinder.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -291,8 +291,8 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth
291291
IntPtr valueList = Runtime.PyDict_Values(kw);
292292
for (int i = 0; i < pynkwargs; ++i)
293293
{
294-
var keyStr = Runtime.GetManagedString(Runtime.PyList_GetItem(keylist, i));
295-
kwargDict[keyStr] = Runtime.PyList_GetItem(valueList, i).DangerousGetAddress();
294+
var keyStr = Runtime.GetManagedString(Runtime.PyList_GetItem(new BorrowedReference(keylist), i));
295+
kwargDict[keyStr] = Runtime.PyList_GetItem(new BorrowedReference(valueList), i).DangerousGetAddress();
296296
}
297297
Runtime.XDecref(keylist);
298298
Runtime.XDecref(valueList);

src/runtime/moduleobject.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ public ManagedType GetAttribute(string name, bool guess)
119119
// cost. Ask the AssemblyManager to do implicit loading for each
120120
// of the steps in the qualified name, then try it again.
121121
bool ignore = name.StartsWith("__");
122-
if (AssemblyManager.LoadImplicit(qname, !ignore))
122+
if (AssemblyManager.LoadImplicit(qname, assemblyLoadErrorHandler: ImportWarning, !ignore))
123123
{
124124
if (AssemblyManager.IsValidNamespace(qname))
125125
{
@@ -161,6 +161,11 @@ public ManagedType GetAttribute(string name, bool guess)
161161
return null;
162162
}
163163

164+
static void ImportWarning(Exception exception)
165+
{
166+
Exceptions.warn(exception.ToString(), Exceptions.ImportWarning);
167+
}
168+
164169

165170
/// <summary>
166171
/// Stores an attribute in the instance dict for future lookups.
@@ -365,7 +370,7 @@ internal void InitializePreload()
365370
if (interactive_preload)
366371
{
367372
interactive_preload = false;
368-
if (Runtime.PySys_GetObject("ps1") != IntPtr.Zero)
373+
if (Runtime.PySys_GetObject("ps1").IsNull)
369374
{
370375
preload = true;
371376
}

src/runtime/pylist.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ public PyList(IntPtr ptr) : base(ptr)
2222
{
2323
}
2424

25+
/// <summary>
26+
/// Creates new <see cref="PyList"/> pointing to the same object, as the given reference.
27+
/// </summary>
28+
internal PyList(BorrowedReference reference) : base(reference) { }
29+
2530

2631
/// <summary>
2732
/// PyList Constructor

src/runtime/pysequence.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ protected PySequence(IntPtr ptr) : base(ptr)
1616
{
1717
}
1818

19+
internal PySequence(BorrowedReference reference) : base(reference) { }
20+
1921
protected PySequence()
2022
{
2123
}

src/runtime/runtime.cs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,9 @@ public class Runtime
111111
// .NET core: System.Runtime.InteropServices.RuntimeInformation.IsOSPlatform(OSPlatform.Windows)
112112
internal static bool IsWindows = Environment.OSVersion.Platform == PlatformID.Win32NT;
113113

114+
internal static Version InteropVersion { get; }
115+
= System.Reflection.Assembly.GetExecutingAssembly().GetName().Version;
116+
114117
static readonly Dictionary<string, OperatingSystemType> OperatingSystemTypeMapping = new Dictionary<string, OperatingSystemType>()
115118
{
116119
{ "Windows", OperatingSystemType.Windows },
@@ -339,9 +342,9 @@ internal static void Initialize(bool initSigs = false)
339342
// Need to add the runtime directory to sys.path so that we
340343
// can find built-in assemblies like System.Data, et. al.
341344
string rtdir = RuntimeEnvironment.GetRuntimeDirectory();
342-
IntPtr path = PySys_GetObject("path");
345+
BorrowedReference path = PySys_GetObject("path");
343346
IntPtr item = PyString_FromString(rtdir);
344-
PyList_Append(new BorrowedReference(path), item);
347+
PyList_Append(path, item);
345348
XDecref(item);
346349
AssemblyManager.UpdatePath();
347350
}
@@ -1642,13 +1645,13 @@ internal static IntPtr PyList_New(long size)
16421645
[DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)]
16431646
internal static extern IntPtr PyList_AsTuple(IntPtr pointer);
16441647

1645-
internal static BorrowedReference PyList_GetItem(IntPtr pointer, long index)
1648+
internal static BorrowedReference PyList_GetItem(BorrowedReference pointer, long index)
16461649
{
16471650
return PyList_GetItem(pointer, new IntPtr(index));
16481651
}
16491652

16501653
[DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)]
1651-
private static extern BorrowedReference PyList_GetItem(IntPtr pointer, IntPtr index);
1654+
private static extern BorrowedReference PyList_GetItem(BorrowedReference pointer, IntPtr index);
16521655

16531656
internal static int PyList_SetItem(IntPtr pointer, long index, IntPtr value)
16541657
{
@@ -1691,13 +1694,13 @@ internal static int PyList_SetSlice(IntPtr pointer, long start, long end, IntPtr
16911694
[DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)]
16921695
private static extern int PyList_SetSlice(IntPtr pointer, IntPtr start, IntPtr end, IntPtr value);
16931696

1694-
internal static long PyList_Size(IntPtr pointer)
1697+
internal static long PyList_Size(BorrowedReference pointer)
16951698
{
16961699
return (long)_PyList_Size(pointer);
16971700
}
16981701

16991702
[DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl, EntryPoint = "PyList_Size")]
1700-
private static extern IntPtr _PyList_Size(IntPtr pointer);
1703+
private static extern IntPtr _PyList_Size(BorrowedReference pointer);
17011704

17021705
//====================================================================
17031706
// Python tuple API
@@ -1825,7 +1828,7 @@ int updatepath
18251828
#endif
18261829

18271830
[DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)]
1828-
internal static extern IntPtr PySys_GetObject(string name);
1831+
internal static extern BorrowedReference PySys_GetObject(string name);
18291832

18301833
[DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)]
18311834
internal static extern int PySys_SetObject(string name, IntPtr ob);
@@ -1923,7 +1926,7 @@ internal static IntPtr PyMem_Realloc(IntPtr ptr, long size)
19231926
internal static extern void PyErr_SetString(IntPtr ob, string message);
19241927

19251928
[DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)]
1926-
internal static extern void PyErr_SetObject(IntPtr ob, IntPtr message);
1929+
internal static extern void PyErr_SetObject(BorrowedReference type, BorrowedReference exceptionObject);
19271930

19281931
[DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)]
19291932
internal static extern IntPtr PyErr_SetFromErrno(IntPtr ob);

0 commit comments

Comments
 (0)