Skip to content

Commit 46a51d4

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 72fae73 commit 46a51d4

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
}
@@ -1653,13 +1656,13 @@ internal static IntPtr PyList_New(long size)
16531656
[DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)]
16541657
internal static extern IntPtr PyList_AsTuple(IntPtr pointer);
16551658

1656-
internal static BorrowedReference PyList_GetItem(IntPtr pointer, long index)
1659+
internal static BorrowedReference PyList_GetItem(BorrowedReference pointer, long index)
16571660
{
16581661
return PyList_GetItem(pointer, new IntPtr(index));
16591662
}
16601663

16611664
[DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)]
1662-
private static extern BorrowedReference PyList_GetItem(IntPtr pointer, IntPtr index);
1665+
private static extern BorrowedReference PyList_GetItem(BorrowedReference pointer, IntPtr index);
16631666

16641667
internal static int PyList_SetItem(IntPtr pointer, long index, IntPtr value)
16651668
{
@@ -1702,13 +1705,13 @@ internal static int PyList_SetSlice(IntPtr pointer, long start, long end, IntPtr
17021705
[DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)]
17031706
private static extern int PyList_SetSlice(IntPtr pointer, IntPtr start, IntPtr end, IntPtr value);
17041707

1705-
internal static long PyList_Size(IntPtr pointer)
1708+
internal static long PyList_Size(BorrowedReference pointer)
17061709
{
17071710
return (long)_PyList_Size(pointer);
17081711
}
17091712

17101713
[DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl, EntryPoint = "PyList_Size")]
1711-
private static extern IntPtr _PyList_Size(IntPtr pointer);
1714+
private static extern IntPtr _PyList_Size(BorrowedReference pointer);
17121715

17131716
//====================================================================
17141717
// Python tuple API
@@ -1836,7 +1839,7 @@ int updatepath
18361839
#endif
18371840

18381841
[DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)]
1839-
internal static extern IntPtr PySys_GetObject(string name);
1842+
internal static extern BorrowedReference PySys_GetObject(string name);
18401843

18411844
[DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)]
18421845
internal static extern int PySys_SetObject(string name, IntPtr ob);
@@ -1934,7 +1937,7 @@ internal static IntPtr PyMem_Realloc(IntPtr ptr, long size)
19341937
internal static extern void PyErr_SetString(IntPtr ob, string message);
19351938

19361939
[DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)]
1937-
internal static extern void PyErr_SetObject(IntPtr ob, IntPtr message);
1940+
internal static extern void PyErr_SetObject(BorrowedReference type, BorrowedReference exceptionObject);
19381941

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

0 commit comments

Comments
 (0)