Skip to content

Commit 5b989cb

Browse files
lostmsulostmsu
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 pythonnet#261 It is an alternative to pythonnet#298
1 parent 3e1fc2e commit 5b989cb

File tree

11 files changed

+110
-73
lines changed

11 files changed

+110
-73
lines changed

‎CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ This document follows the conventions laid out in [Keep a CHANGELOG][].
1111

1212
### Changed
1313
- Drop support for Python 2
14+
- `clr.AddReference` may now throw errors besides `FileNotFoundException`, that provide more
15+
details about the cause of the failure
16+
- `clr.AddReference` no longer adds ".dll" implicitly
1417

1518
### Fixed
1619

‎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: 24 additions & 41 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,14 @@ 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))
249-
{
250-
name = name + ".dll";
251-
}
252232
if (File.Exists(name))
253233
{
254-
try
255-
{
256-
assembly = Assembly.LoadFrom(name);
257-
}
258-
catch (Exception)
259-
{
260-
}
234+
return Assembly.LoadFrom(name);
261235
}
262236
}
263-
return assembly;
237+
return null;
264238
}
265239

266240
/// <summary>
@@ -291,7 +265,7 @@ public static Assembly FindLoadedAssembly(string name)
291265
/// actually loads an assembly.
292266
/// Call ONLY for namespaces that HAVE NOT been cached yet.
293267
/// </remarks>
294-
public static bool LoadImplicit(string name, bool warn = true)
268+
public static bool LoadImplicit(string name, Action<Exception> assemblyLoadErrorHandler, bool warn = true)
295269
{
296270
string[] names = name.Split('.');
297271
var loaded = false;
@@ -308,14 +282,23 @@ public static bool LoadImplicit(string name, bool warn = true)
308282
assembliesSet = new HashSet<Assembly>(AppDomain.CurrentDomain.GetAssemblies());
309283
}
310284
Assembly a = FindLoadedAssembly(s);
311-
if (a == null)
312-
{
313-
a = LoadAssemblyPath(s);
314-
}
315-
if (a == null)
285+
try
316286
{
317-
a = LoadAssembly(s);
287+
if (a == null)
288+
{
289+
a = LoadAssemblyPath(s);
290+
}
291+
292+
if (a == null)
293+
{
294+
a = LoadAssembly(s);
295+
}
318296
}
297+
catch (FileLoadException e) { assemblyLoadErrorHandler(e); }
298+
catch (BadImageFormatException e) { assemblyLoadErrorHandler(e); }
299+
catch (System.Security.SecurityException e) { assemblyLoadErrorHandler(e); }
300+
catch (PathTooLongException e) { assemblyLoadErrorHandler(e); }
301+
319302
if (a != null && !assembliesSet.Contains(a))
320303
{
321304
loaded = true;

‎src/runtime/exceptions.cs

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

262262
/// <summary>
@@ -286,7 +286,7 @@ public static void SetError(Exception e)
286286

287287
IntPtr op = CLRObject.GetInstHandle(e);
288288
IntPtr etype = Runtime.PyObject_GetAttrString(op, "__class__");
289-
Runtime.PyErr_SetObject(etype, op);
289+
Runtime.PyErr_SetObject(new BorrowedReference(etype), new BorrowedReference(op));
290290
Runtime.XDecref(etype);
291291
Runtime.XDecref(op);
292292
}

‎src/runtime/importhook.cs

Lines changed: 19 additions & 15 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
@@ -222,22 +223,12 @@ public static IntPtr __import__(IntPtr self, IntPtr args, IntPtr kw)
222223
// Check these BEFORE the built-in import runs; may as well
223224
// do the Incref()ed return here, since we've already found
224225
// the module.
225-
if (mod_name == "clr")
226+
if (mod_name == "clr" || mod_name == "CLR")
226227
{
227-
IntPtr clr_module = GetCLRModule(fromList);
228-
if (clr_module != IntPtr.Zero)
228+
if (mod_name == "CLR")
229229
{
230-
IntPtr sys_modules = Runtime.PyImport_GetModuleDict();
231-
if (sys_modules != IntPtr.Zero)
232-
{
233-
Runtime.PyDict_SetItemString(sys_modules, "clr", clr_module);
234-
}
230+
Exceptions.deprecation("The CLR module is deprecated. Please use 'clr'.");
235231
}
236-
return clr_module;
237-
}
238-
if (mod_name == "CLR")
239-
{
240-
Exceptions.deprecation("The CLR module is deprecated. Please use 'clr'.");
241232
IntPtr clr_module = GetCLRModule(fromList);
242233
if (clr_module != IntPtr.Zero)
243234
{
@@ -249,8 +240,10 @@ public static IntPtr __import__(IntPtr self, IntPtr args, IntPtr kw)
249240
}
250241
return clr_module;
251242
}
243+
252244
string realname = mod_name;
253245
string clr_prefix = null;
246+
254247
if (mod_name.StartsWith("CLR."))
255248
{
256249
clr_prefix = "CLR."; // prepend when adding the module to sys.modules
@@ -312,11 +305,22 @@ public static IntPtr __import__(IntPtr self, IntPtr args, IntPtr kw)
312305
AssemblyManager.UpdatePath();
313306
if (!AssemblyManager.IsValidNamespace(realname))
314307
{
315-
if (!AssemblyManager.LoadImplicit(realname))
308+
var loadExceptions = new List<Exception>();
309+
if (!AssemblyManager.LoadImplicit(realname, assemblyLoadErrorHandler: loadExceptions.Add))
316310
{
317311
// May be called when a module being imported imports a module.
318312
// In particular, I've seen decimal import copy import org.python.core
319-
return Runtime.PyObject_Call(py_import, args, kw);
313+
IntPtr importResult = Runtime.PyObject_Call(py_import, args, kw);
314+
// TODO: use ModuleNotFoundError in Python 3.6+
315+
if (importResult == IntPtr.Zero && loadExceptions.Count > 0
316+
&& Exceptions.ExceptionMatches(Exceptions.ImportError))
317+
{
318+
loadExceptions.Add(new PythonException());
319+
var importError = new PyObject(new BorrowedReference(Exceptions.ImportError));
320+
importError.SetAttr("__cause__", new AggregateException(loadExceptions).ToPython());
321+
Runtime.PyErr_SetObject(new BorrowedReference(Exceptions.ImportError), importError.Reference);
322+
}
323+
return importResult;
320324
}
321325
}
322326

‎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/pytuple.cs

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

25+
/// <summary>
26+
/// PyTuple Constructor
27+
/// </summary>
28+
/// <remarks>
29+
/// Creates a new PyTuple from an existing object reference.
30+
/// The object reference is not checked for type-correctness.
31+
/// </remarks>
32+
internal PyTuple(BorrowedReference reference) : base(reference) { }
33+
2534

2635
/// <summary>
2736
/// PyTuple Constructor

0 commit comments

Comments
 (0)