FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Merge pull request #1973 from losttech/bugs/1972 · pythonnet/pythonnet@ce76dae · GitHub

/ pythonnet Public

Commit ce76dae

Browse files
filmor
authored
Merge pull request #1973 from losttech/bugs/1972
Delete target object from event handler collections when it has no more event handlers
2 parents bf984f0 + 5a28fd4 commit ce76dae

6 files changed

Lines changed: 110 additions & 5 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ This document follows the conventions laid out in [Keep a CHANGELOG][].
1313

1414
### Fixed
1515

16+
- Fixed objects leaking when Python attached event handlers to them even if they were later removed
17+
1618

1719
## [3.0.0](https://github.com/pythonnet/pythonnet/releases/tag/v3.0.0) - 2022-09-29
1820

‎src/embed_tests/Events.cs‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
using System;
2+
using System.Diagnostics;
3+
using System.Threading;
4+
5+
using NUnit.Framework;
6+
7+
using Python.Runtime;
8+
9+
namespace Python.EmbeddingTest;
10+
11+
public class Events
12+
{
13+
[OneTimeSetUp]
14+
public void SetUp()
15+
{
16+
PythonEngine.Initialize();
17+
}
18+
19+
[OneTimeTearDown]
20+
public void Dispose()
21+
{
22+
PythonEngine.Shutdown();
23+
}
24+
25+
[Test]
26+
public void UsingDoesNotLeak()
27+
{
28+
using var scope = Py.CreateScope();
29+
scope.Exec(@"
30+
import gc
31+
32+
from Python.EmbeddingTest import ClassWithEventHandler
33+
34+
def event_handler():
35+
pass
36+
37+
for _ in range(2000):
38+
example = ClassWithEventHandler()
39+
example.LeakEvent += event_handler
40+
example.LeakEvent -= event_handler
41+
del example
42+
43+
gc.collect()
44+
");
45+
Runtime.Runtime.TryCollectingGarbage(10);
46+
Assert.AreEqual(0, ClassWithEventHandler.alive);
47+
}
48+
}
49+
50+
public class ClassWithEventHandler
51+
{
52+
internal static int alive;
53+
54+
public event EventHandler LeakEvent;
55+
private Array arr; // dummy array to exacerbate memory leak
56+
57+
public ClassWithEventHandler()
58+
{
59+
Interlocked.Increment(ref alive);
60+
this.arr = new int[800];
61+
}
62+
63+
~ClassWithEventHandler()
64+
{
65+
Interlocked.Decrement(ref alive);
66+
}
67+
}

‎src/runtime/Finalizer.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,8 @@ struct PendingFinalization
364364
{
365365
public IntPtr PyObj;
366366
public BorrowedReference Ref => new(PyObj);
367+
public ManagedType? Managed => ManagedType.GetManagedObject(Ref);
368+
public nint RefCount => Runtime.Refcount(Ref);
367369
public int RuntimeRun;
368370
#if TRACE_ALLOC
369371
public string StackTrace;

‎src/runtime/PythonTypes/PyObject.cs‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1051,9 +1051,20 @@ public PyList Dir()
10511051
return Runtime.GetManagedString(strval.BorrowOrThrow());
10521052
}
10531053

1054-
string? DebuggerDisplay => DebugUtil.HaveInterpreterLock()
1055-
? this.ToString()
1056-
: $"pyobj at 0x{this.rawPtr:X} (get Py.GIL to see more info)";
1054+
ManagedType? InternalManagedObject => ManagedType.GetManagedObject(this.Reference);
1055+
1056+
string? DebuggerDisplay
1057+
{
1058+
get
1059+
{
1060+
if (DebugUtil.HaveInterpreterLock())
1061+
return this.ToString();
1062+
var obj = this.InternalManagedObject;
1063+
return obj is { }
1064+
? obj.ToString()
1065+
: $"pyobj at 0x{this.rawPtr:X} (get Py.GIL to see more info)";
1066+
}
1067+
}
10571068

10581069

10591070
/// <summary>

‎src/runtime/Util/CodeGenerator.cs‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
using System;
22
using System.Collections.Generic;
3+
using System.Globalization;
4+
using System.Linq;
35
using System.Reflection;
46
using System.Reflection.Emit;
57
using System.Threading;
@@ -17,13 +19,15 @@ internal class CodeGenerator
1719
private readonly AssemblyBuilder aBuilder;
1820
private readonly ModuleBuilder mBuilder;
1921

22+
const string NamePrefix = "__Python_Runtime_Generated_";
23+
2024
internal CodeGenerator()
2125
{
22-
var aname = new AssemblyName { Name = "__CodeGenerator_Assembly" };
26+
var aname = new AssemblyName { Name = GetUniqueAssemblyName(NamePrefix + "Assembly") };
2327
var aa = AssemblyBuilderAccess.Run;
2428

2529
aBuilder = Thread.GetDomain().DefineDynamicAssembly(aname, aa);
26-
mBuilder = aBuilder.DefineDynamicModule("__CodeGenerator_Module");
30+
mBuilder = aBuilder.DefineDynamicModule(NamePrefix + "Module");
2731
}
2832

2933
/// <summary>
@@ -77,5 +81,20 @@ internal static void GenerateMarshalByRefsBack(ILGenerator il, IReadOnlyList<Typ
7781
}
7882
}
7983
}
84+
85+
static string GetUniqueAssemblyName(string name)
86+
{
87+
var taken = new HashSet<string>(AppDomain.CurrentDomain
88+
.GetAssemblies()
89+
.Select(a => a.GetName().Name));
90+
for (int i = 0; i < int.MaxValue; i++)
91+
{
92+
string candidate = name + i.ToString(CultureInfo.InvariantCulture);
93+
if (!taken.Contains(candidate))
94+
return candidate;
95+
}
96+
97+
throw new NotSupportedException("Too many assemblies");
98+
}
8099
}
81100
}

‎src/runtime/Util/EventHandlerCollection.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,10 @@ internal bool RemoveEventHandler(BorrowedReference target, BorrowedReference han
9999
continue;
100100
}
101101
list.RemoveAt(i);
102+
if (list.Count == 0)
103+
{
104+
Remove(key);
105+
}
102106
return true;
103107
}
104108

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL