diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 2a4a74f11..e454bb8f3 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -76,6 +76,14 @@ jobs: Out-File -FilePath $env:GITHUB_ENV -Encoding utf8 -Append -InputObject "PYTHONNET_PYDLL=$(python -m find_libpython)" Out-File -FilePath $env:GITHUB_ENV -Encoding utf8 -Append -InputObject "PYTHONHOME=$(python -c 'import sys; print(sys.prefix)')" + - name: Install .NET 6.0 x86 runtime + if: ${{ matrix.os.category == 'windows' && matrix.os.platform == 'x86' }} + run: | + $installScript = "$env:TEMP\dotnet-install.ps1" + Invoke-WebRequest -Uri "https://dot.net/v1/dotnet-install.ps1" -OutFile $installScript + & $installScript -Channel "6.0" -Architecture "x86" -InstallDir "${env:ProgramFiles(x86)}\dotnet" -Runtime "dotnet" + Remove-Item $installScript + - name: Embedding tests run: dotnet test --runtime any-${{ matrix.os.platform }} --logger "console;verbosity=detailed" src/embed_tests/ env: diff --git a/CHANGELOG.md b/CHANGELOG.md index 52b5a0f99..ace09add3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ This document follows the conventions laid out in [Keep a CHANGELOG][]. ## [Unreleased][] +### Fixed + +- Fixed an issue where a concrete type object implementing an IDisposable when returned from a method within a `with` statement, for example, was incorrectly resolved as the IDisposable interface type instead of the concrete type. + ### Added - Added support for hiding members from inherited classes. diff --git a/Directory.Build.props b/Directory.Build.props index e45c16f6a..b3d933914 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -16,9 +16,9 @@ all runtime; build; native; contentfiles; analyzers - + diff --git a/src/embed_tests/Python.EmbeddingTest.csproj b/src/embed_tests/Python.EmbeddingTest.csproj index 4993994d3..4ecb689d7 100644 --- a/src/embed_tests/Python.EmbeddingTest.csproj +++ b/src/embed_tests/Python.EmbeddingTest.csproj @@ -24,7 +24,7 @@ all runtime; build; native; contentfiles; analyzers; buildtransitive - + 1.0.0 all diff --git a/src/embed_tests/TestConverter.cs b/src/embed_tests/TestConverter.cs index a59b9c97b..83540a16a 100644 --- a/src/embed_tests/TestConverter.cs +++ b/src/embed_tests/TestConverter.cs @@ -209,6 +209,90 @@ class PyGetListImpl(test.GetListImpl): List result = inst.GetList(); CollectionAssert.AreEqual(new[] { "testing" }, result); } + + /// + /// Test that when a method returns a concrete type implementing IDisposable, + /// the object is wrapped as the concrete type (not IDisposable interface), + /// preserving access to concrete type members and supporting 'with' statements. + /// + [Test] + public void ConcreteTypeImplementingIDisposable_IsWrappedAsConcreteType() + { + using var scope = Py.CreateScope(); + scope.Import(typeof(ConcreteDisposableResource).Namespace, asname: "test"); + + // Reset static state + ConcreteDisposableResource.IsDisposed = false; + ConcreteDisposableResource.InstanceCount = 0; + + // Test that a method returning IDisposable but actually returning a concrete type + // wraps the object as the concrete type, not the interface + scope.Exec(@" +import clr +clr.AddReference('Python.EmbeddingTest') +from Python.EmbeddingTest import ConcreteDisposableResource + +# Get a resource through a method that declares IDisposable return type +resource = ConcreteDisposableResource.GetResource() + +# Verify it's wrapped as the concrete type, not IDisposable +# The concrete type has a GetValue() method that IDisposable doesn't have +value = resource.GetValue() +assert value == 42, f'Expected 42, got {value}' + +# Verify the concrete type name is accessible +type_name = resource.GetType().Name +assert type_name == 'ConcreteDisposableResource', f'Expected ConcreteDisposableResource, got {type_name}' + +# Verify 'with' statement still works (IDisposable support) +with resource: + inside_value = resource.GetValue() + assert inside_value == 42 + assert ConcreteDisposableResource.IsDisposed == False + +# After 'with' block, should be disposed +assert ConcreteDisposableResource.IsDisposed == True +"); + + // Verify the resource was actually disposed + Assert.IsTrue(ConcreteDisposableResource.IsDisposed, "Resource should be disposed after 'with' statement"); + } + + /// + /// Test that Converter.ToPython wraps concrete types implementing interfaces + /// as the concrete type, not the interface, when the declared type is an interface. + /// + [Test] + public void Converter_ToPython_ConcreteTypeOverInterface() + { + using (Py.GIL()) + { + // Create a concrete type that implements IDisposable + var concreteResource = new ConcreteDisposableResource(100); + + // Convert using IDisposable as the declared type (simulating method return type) + var pyObject = Converter.ToPython(concreteResource, typeof(IDisposable)); + + // Verify it's wrapped as the concrete type, not IDisposable + var wrappedObject = ManagedType.GetManagedObject(pyObject.BorrowOrThrow()); + Assert.IsInstanceOf(wrappedObject); + + var clrObject = (CLRObject)wrappedObject; + var wrappedType = clrObject.inst.GetType(); + + // Should be the concrete type, not IDisposable + Assert.AreEqual(typeof(ConcreteDisposableResource), wrappedType); + Assert.AreNotEqual(typeof(IDisposable), wrappedType); + + // Verify we can access concrete type members from Python + using var scope = Py.CreateScope(); + scope.Set("resource", pyObject.MoveToPyObject()); + var result = scope.Eval("resource.GetValue()"); + Assert.AreEqual(100, result.As()); + + pyObject.Dispose(); + } + } } public interface IGetList @@ -220,4 +304,44 @@ public class GetListImpl : IGetList { public List GetList() => new() { "testing" }; } + + /// + /// A concrete class implementing IDisposable with additional members. + /// Used to test that methods returning IDisposable but actually returning + /// concrete types are wrapped as the concrete type, not the interface. + /// + public class ConcreteDisposableResource : IDisposable + { + public static bool IsDisposed { get; set; } + public static int InstanceCount { get; set; } + + private readonly int _value; + + public ConcreteDisposableResource(int value = 42) + { + _value = value; + InstanceCount++; + IsDisposed = false; + } + + /// + /// A method that exists only on the concrete type, not on IDisposable. + /// This verifies that the object is wrapped as the concrete type. + /// + public int GetValue() => _value; + + public void Dispose() + { + IsDisposed = true; + } + + /// + /// A method that declares IDisposable return type but actually returns + /// the concrete type. This is the scenario we're testing. + /// + public static IDisposable GetResource() + { + return new ConcreteDisposableResource(); + } + } } diff --git a/src/module_tests/Python.ModuleTest.csproj b/src/module_tests/Python.ModuleTest.csproj index 7a8aa9ac3..081b8d871 100644 --- a/src/module_tests/Python.ModuleTest.csproj +++ b/src/module_tests/Python.ModuleTest.csproj @@ -20,7 +20,7 @@ all runtime; build; native; contentfiles; analyzers; buildtransitive - + 1.0.0 all diff --git a/src/perf_tests/Python.PerformanceTests.csproj b/src/perf_tests/Python.PerformanceTests.csproj index b526183cc..5a5ab5455 100644 --- a/src/perf_tests/Python.PerformanceTests.csproj +++ b/src/perf_tests/Python.PerformanceTests.csproj @@ -5,16 +5,10 @@ false x64 x64 - - - - ..\..\pythonnet\runtime\Python.Runtime.dll - true - + - diff --git a/src/python_tests_runner/Python.PythonTestsRunner.csproj b/src/python_tests_runner/Python.PythonTestsRunner.csproj index 63981c424..d5cf106d1 100644 --- a/src/python_tests_runner/Python.PythonTestsRunner.csproj +++ b/src/python_tests_runner/Python.PythonTestsRunner.csproj @@ -11,7 +11,7 @@ - + all runtime; build; native; contentfiles; analyzers; buildtransitive diff --git a/src/runtime/Converter.cs b/src/runtime/Converter.cs index c7154ce36..3aca907e7 100644 --- a/src/runtime/Converter.cs +++ b/src/runtime/Converter.cs @@ -142,8 +142,20 @@ internal static NewReference ToPython(object? value, Type type) if (type.IsInterface) { - var ifaceObj = (InterfaceObject)ClassManager.GetClassImpl(type); - return ifaceObj.TryWrapObject(value); + Type actualType = value.GetType(); + // This fixes issues where methods return concrete types that implement + // interfaces (e.g., IDisposable) but should be exposed as their + // concrete type for proper member access and Python 'with' statement support. + if (!actualType.IsInterface && type == typeof(IDisposable) && typeof(IDisposable).IsAssignableFrom(actualType)) + { + type = actualType; + } + else + { + // Runtime type is also an interface (rare: proxy/dynamic case), wrap as interface + var ifaceObj = (InterfaceObject)ClassManager.GetClassImpl(type); + return ifaceObj.TryWrapObject(value); + } } if (type.IsArray || type.IsEnum)