Skip to content

Commit 37df647

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 #261 It is an alternative to #298
1 parent 3e1fc2e commit 37df647

File tree

11 files changed

+141
-113
lines changed

11 files changed

+141
-113
lines changed

‎CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ 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
17+
- the old `CLR` module prefix is gone
1418

1519
### Fixed
1620

‎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: 49 additions & 55 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,53 +240,45 @@ 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;
254-
if (mod_name.StartsWith("CLR."))
255-
{
256-
clr_prefix = "CLR."; // prepend when adding the module to sys.modules
257-
realname = mod_name.Substring(4);
258-
string msg = $"Importing from the CLR.* namespace is deprecated. Please import '{realname}' directly.";
259-
Exceptions.deprecation(msg);
260-
}
261-
else
246+
247+
// 2010-08-15: Always seemed smart to let python try first...
248+
// This shaves off a few tenths of a second on test_module.py
249+
// and works around a quirk where 'sys' is found by the
250+
// LoadImplicit() deprecation logic.
251+
// Turns out that the AssemblyManager.ResolveHandler() checks to see if any
252+
// Assembly's FullName.ToLower().StartsWith(name.ToLower()), which makes very
253+
// little sense to me.
254+
IntPtr res = Runtime.PyObject_Call(py_import, args, kw);
255+
if (res != IntPtr.Zero)
262256
{
263-
// 2010-08-15: Always seemed smart to let python try first...
264-
// This shaves off a few tenths of a second on test_module.py
265-
// and works around a quirk where 'sys' is found by the
266-
// LoadImplicit() deprecation logic.
267-
// Turns out that the AssemblyManager.ResolveHandler() checks to see if any
268-
// Assembly's FullName.ToLower().StartsWith(name.ToLower()), which makes very
269-
// little sense to me.
270-
IntPtr res = Runtime.PyObject_Call(py_import, args, kw);
271-
if (res != IntPtr.Zero)
272-
{
273-
// There was no error.
274-
if (fromlist && IsLoadAll(fromList))
275-
{
276-
var mod = ManagedType.GetManagedObject(res) as ModuleObject;
277-
mod?.LoadNames();
278-
}
279-
return res;
280-
}
281-
// There was an error
282-
if (!Exceptions.ExceptionMatches(Exceptions.ImportError))
257+
// There was no error.
258+
if (fromlist && IsLoadAll(fromList))
283259
{
284-
// and it was NOT an ImportError; bail out here.
285-
return IntPtr.Zero;
260+
var mod = ManagedType.GetManagedObject(res) as ModuleObject;
261+
mod?.LoadNames();
286262
}
263+
return res;
264+
}
265+
// There was an error
266+
if (!Exceptions.ExceptionMatches(Exceptions.ImportError))
267+
{
268+
// and it was NOT an ImportError; bail out here.
269+
return IntPtr.Zero;
270+
}
287271

288-
if (mod_name == string.Empty)
289-
{
290-
// Most likely a missing relative import.
291-
// For example site-packages\bs4\builder\__init__.py uses it to check if a package exists:
292-
// from . import _html5lib
293-
// We don't support them anyway
294-
return IntPtr.Zero;
295-
}
296-
// Otherwise, just clear the it.
297-
Exceptions.Clear();
272+
if (mod_name == string.Empty)
273+
{
274+
// Most likely a missing relative import.
275+
// For example site-packages\bs4\builder\__init__.py uses it to check if a package exists:
276+
// from . import _html5lib
277+
// We don't support them anyway
278+
return IntPtr.Zero;
298279
}
280+
// Otherwise, just clear the it.
281+
Exceptions.Clear();
299282

300283
string[] names = realname.Split('.');
301284

@@ -312,11 +295,22 @@ public static IntPtr __import__(IntPtr self, IntPtr args, IntPtr kw)
312295
AssemblyManager.UpdatePath();
313296
if (!AssemblyManager.IsValidNamespace(realname))
314297
{
315-
if (!AssemblyManager.LoadImplicit(realname))
298+
var loadExceptions = new List<Exception>();
299+
if (!AssemblyManager.LoadImplicit(realname, assemblyLoadErrorHandler: loadExceptions.Add))
316300
{
317301
// May be called when a module being imported imports a module.
318302
// In particular, I've seen decimal import copy import org.python.core
319-
return Runtime.PyObject_Call(py_import, args, kw);
303+
IntPtr importResult = Runtime.PyObject_Call(py_import, args, kw);
304+
// TODO: use ModuleNotFoundError in Python 3.6+
305+
if (importResult == IntPtr.Zero && loadExceptions.Count > 0
306+
&& Exceptions.ExceptionMatches(Exceptions.ImportError))
307+
{
308+
loadExceptions.Add(new PythonException());
309+
var importError = new PyObject(new BorrowedReference(Exceptions.ImportError));
310+
importError.SetAttr("__cause__", new AggregateException(loadExceptions).ToPython());
311+
Runtime.PyErr_SetObject(new BorrowedReference(Exceptions.ImportError), importError.Reference);
312+
}
313+
return importResult;
320314
}
321315
}
322316

‎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
}

0 commit comments

Comments
 (0)