Repository navigation
List conversion fails on new version of pythonnet compared to 3.0.0.post1 #2004
Description
Environment
-
Pythonnet version:
Works with pythonnet==3.0.0.post1
Failes with pythonnet>=3.0.0.post1 -
Python version:
Python 3.8.0 -
Operating System:
Windows -
.NET Runtime:
netstandard2.0;net471
Details
I followed the notes to write a codec to convert a C# List to a Python list, here it is:
/// <summary>
/// This class will convert a C# list of strings to a Python list [] of strings.
/// </summary>
public class ListStringConversions : IPyObjectEncoder
{
bool IPyObjectEncoder.CanEncode(Type type)
{
return type == typeof(IList<string>);
}
PyObject IPyObjectEncoder.TryEncode(object value)
{
using (var scope = Py.CreateScope())
{
scope.Exec($"a = []");
var input = (List<string>)value;
foreach (var item in input)
scope.Exec($"a.append('{item}')"); // {item}
return scope.Get("a");
}
}
}
I have a simple test which calls in to a C# assembly and returns a list. With pythonnet version 3.0.0.post1, the C# list is successfully converted to a python list.
In my requirements file, I had this
pythonnet>=3.0.0.post1
Now on the CICD process it has updated the pythonnet package and the same test fails:
> assert isinstance(codes, list)
E assert False
E + where False = isinstance(<System.Collections.Generic.IList[String] object at 0x0000023C1D9D5180>, list)
The code no longer is returning a python list. I switched the requirements file to
pythonnet==3.0.0.post1
and the test passes. As such I think there is a breaking change in the latest release that prevents the conversions been executed.
Activity
filmor commentedon Nov 7, 2022
What is the actual test that you are running here?
screig commentedon Nov 7, 2022
filmor commentedon Nov 7, 2022
Can you please boil this down to a reproducible example? The potentially relevant change is #1986, but I can't verify that like this.
(As a side note, this way of implementing the conversion is a recipe for disaster. Imagine someone gets the string
"'); import os; os.system('rm -rf /'); (')"into any list that is returned from C#).screig commentedon Nov 7, 2022
Well I would love to see more examples on conversions, but there seem to be very thin on the ground
lostmsu commentedon Nov 7, 2022
This never could have worked because no concrete instance has type of
IList<string>.screig commentedon Nov 7, 2022
No that still fails, even with this change, I will make you a stand alone test later (am at work)
screig commentedon Nov 8, 2022
I made you a sample project
https://github.com/screig/pythonnet_test
It runs the tests under Github actions, there are two jobs. Which are identical apart from, one does this
and the other, does this:
https://github.com/screig/pythonnet_test/actions/runs/3418546969
As you can see 3.0.1 fails
filmor commentedon Nov 8, 2022
Great, thank you for the example, I'll look into it.
filmor commentedon Nov 9, 2022
Wow, this is a weird one:
The issue is due to the fact that you reference Python.NET 3.0.0 in your project explicitly. This leads to your
RegisterEncodercall being run in the 3.0.0 instance, while the version loaded by Python (3.0.1) looks up the encoders in its own instance of the respective variable. We have to prevent loading of multiple instances ofPython.Runtime.dll, I'll see if I can implement that.filmor commentedon Nov 9, 2022
It's actually enough to just delete the
Python.Runtime.dllfrom your lib directory. Instead of doing a manual copy,dotnet publishthe project to alibdirectory and referencepythonnetlike this:I'll update the documentation accordingly. @lostmsu, are you aware of a way to prevent loading multiple versions of a DLL?
lostmsu commentedon Nov 9, 2022
@filmor can always scan the loaded assemblies like we do for reflecting types. Just skip the assemblies loaded in reflection only context.
filmor commentedon Nov 10, 2022
I don't know what you mean. User assemblies are just loaded using
Assembly.Load, which resolves the dependencies automatically and (to my knowledge) without an option to modify behaviour. We could consider splitting up into a "loader" DLL and the actual Runtime, ensuring that all globals are registered in the loader if available (this is also relevant for our instance tracking). The simplest solution would of course be to just prevent the .NET runtime from loading the second version. We could also build separate nuget packages for both scenarios.(By the way, I just tested it out, this is only a problem for .NET Framework/Mono, .NET Core only loads the first version it finds)
lostmsu commentedon Nov 10, 2022
@filmor we also scan already loaded assemblies. E.g.
AppDomain.CurrentDomain.GetAssemblies()filmor commentedon Nov 10, 2022
How does that help?
lostmsu commentedon Nov 10, 2022
@filmor upon initialization of Python.NET you can call that method and ensure there is only one Python.Runtime loaded, or throw otherwise.
filmor commentedon Nov 10, 2022
Python.NET is already initialized at that point.
lostmsu commentedon Nov 10, 2022
This check can go in many places. For instance, you could use
PyObjectstatic constructor. Or you could add this check toAssemblyManager.ScanAssemblyfilmor commentedon Nov 10, 2022
The only thing we could do at that point would be to throw an exception, though. I don't think there is a way to fix the situation once the second instance is loaded.
lostmsu commentedon Nov 10, 2022
I don't think there's anything for us to fix here. This is an unsupported situation. Detect and raise.