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

Fixed CollectBasicObject test by lostmsu · Pull Request #1313 · pythonnet/pythonnet · GitHub

/ pythonnet Public

Fixed CollectBasicObject test #1313

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
filmor merged 3 commits into from
Dec 12, 2020
Conversation 4 Commits 3 Checks 0 Files changed
Merged

Fixed CollectBasicObject test #1313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d888244
fixed CollectBasicObject by ensuring MakeAGarbage is not keeping temp…
lostmsu Dec 10, 2020
ff86507
reworked Finalizer to only store raw python object handles instead of…
lostmsu Dec 11, 2020
ca96444
fixed domain reload test, that tries to call static method from objec…
lostmsu Dec 11, 2020
File filter

Filter by extension

Filter by extension .cs  (10) All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
6 changes: 3 additions & 3 deletions src/embed_tests/TestDomainReload.cs
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ from Python.EmbeddingTest.Domain import MyClass
def test_obj_call():
obj = MyClass()
obj.Method()
obj.StaticMethod()
MyClass.StaticMethod()
obj.Property = 1
obj.Field = 10

Expand Down Expand Up @@ -288,7 +288,7 @@ void ExecTest()

GC.Collect();
GC.WaitForPendingFinalizers(); // <- this will put former `num` into Finalizer queue
Finalizer.Instance.Collect(forceDispose: true);
Finalizer.Instance.Collect();
// ^- this will call PyObject.Dispose, which will call XDecref on `num.Handle`,
// but Python interpreter from "run" 1 is long gone, so it will corrupt memory instead.
Assert.False(numRef.IsAlive);
Expand Down Expand Up @@ -333,7 +333,7 @@ void ExecTest()
PythonEngine.Initialize(); // <- "run" 2 starts
GC.Collect();
GC.WaitForPendingFinalizers();
Finalizer.Instance.Collect(forceDispose: true);
Finalizer.Instance.Collect();
Assert.False(objRef.IsAlive);
}
finally
Expand Down
149 changes: 33 additions & 116 deletions src/embed_tests/TestFinalizer.cs
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
using Python.Runtime;
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Diagnostics;
using System.Linq;
using System.Runtime.CompilerServices;
using System.Threading;

namespace Python.EmbeddingTest
Expand All @@ -28,26 +28,14 @@ public void TearDown()
PythonEngine.Shutdown();
}

private static bool FullGCCollect()
private static void FullGCCollect()
{
GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced);
try
{
return GC.WaitForFullGCComplete() == GCNotificationStatus.Succeeded;
}
catch (NotImplementedException)
{
// Some clr runtime didn't implement GC.WaitForFullGCComplete yet.
return false;
}
finally
{
GC.WaitForPendingFinalizers();
}
GC.Collect();
GC.WaitForPendingFinalizers();
}

[Test]
[Ignore("Ignore temporarily")]
[Obsolete("GC tests are not guaranteed")]
public void CollectBasicObject()
{
Assert.IsTrue(Finalizer.Instance.Enable);
Expand All @@ -64,11 +52,7 @@ public void CollectBasicObject()
Assert.IsFalse(called, "The event handler was called before it was installed");
Finalizer.Instance.CollectOnce += handler;

WeakReference shortWeak;
WeakReference longWeak;
{
MakeAGarbage(out shortWeak, out longWeak);
}
IntPtr pyObj = MakeAGarbage(out var shortWeak, out var longWeak);
FullGCCollect();
// The object has been resurrected
Warn.If(
Expand All @@ -86,7 +70,7 @@ public void CollectBasicObject()
var garbage = Finalizer.Instance.GetCollectedObjects();
Assert.NotZero(garbage.Count, "There should still be garbage around");
Warn.Unless(
garbage.Any(T => ReferenceEquals(T.Target, longWeak.Target)),
garbage.Contains(pyObj),
$"The {nameof(longWeak)} reference doesn't show up in the garbage list",
garbage
);
Expand All @@ -104,33 +88,45 @@ public void CollectBasicObject()
}

[Test]
[Ignore("Ignore temporarily")]
[Obsolete("GC tests are not guaranteed")]

filmor Dec 11, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Is NUnit aware of this attribute or does this just generate yet another compile-time warning? Does it help to retry the test using https://docs.nunit.org/articles/nunit/writing-tests/attributes/retry.html?

lostmsu Dec 11, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

NUnit is not aware. This actually removes warning from the usage of other [Obsolete] code.
public void CollectOnShutdown()
{
IntPtr op = MakeAGarbage(out var shortWeak, out var longWeak);
int hash = shortWeak.Target.GetHashCode();
List<WeakReference> garbage;
if (!FullGCCollect())
{
Assert.IsTrue(WaitForCollected(op, hash, 10000));
}
FullGCCollect();
Assert.IsFalse(shortWeak.IsAlive);
garbage = Finalizer.Instance.GetCollectedObjects();
List<IntPtr> garbage = Finalizer.Instance.GetCollectedObjects();
Assert.IsNotEmpty(garbage, "The garbage object should be collected");
Assert.IsTrue(garbage.Any(r => ReferenceEquals(r.Target, longWeak.Target)),
Assert.IsTrue(garbage.Contains(op),
"Garbage should contains the collected object");

PythonEngine.Shutdown();
garbage = Finalizer.Instance.GetCollectedObjects();
Assert.IsEmpty(garbage);
}

[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)] // ensure lack of references to obj
[Obsolete("GC tests are not guaranteed")]
private static IntPtr MakeAGarbage(out WeakReference shortWeak, out WeakReference longWeak)
{
PyLong obj = new PyLong(1024);
shortWeak = new WeakReference(obj);
longWeak = new WeakReference(obj, true);
return obj.Handle;
IntPtr handle = IntPtr.Zero;
WeakReference @short = null, @long = null;
// must create Python object in the thread where we have GIL
IntPtr val = PyLong.FromLong(1024);
// must create temp object in a different thread to ensure it is not present
// when conservatively scanning stack for GC roots.
// see https://xamarin.github.io/bugzilla-archives/17/17593/bug.html
var garbageGen = new Thread(() =>
{
var obj = new PyObject(val, skipCollect: true);
@short = new WeakReference(obj);
@long = new WeakReference(obj, true);
handle = obj.Handle;
});
garbageGen.Start();
Assert.IsTrue(garbageGen.Join(TimeSpan.FromSeconds(5)), "Garbage creation timed out");
shortWeak = @short;
longWeak = @long;
return handle;
}

private static long CompareWithFinalizerOn(PyObject pyCollect, bool enbale)
Expand Down Expand Up @@ -191,62 +187,6 @@ public void SimpleTestMemory()
}
}

class MyPyObject : PyObject
{
public MyPyObject(IntPtr op) : base(op)
{
}

protected override void Dispose(bool disposing)
{
base.Dispose(disposing);
GC.SuppressFinalize(this);
throw new Exception("MyPyObject");
}
internal static void CreateMyPyObject(IntPtr op)
{
Runtime.Runtime.XIncref(op);
new MyPyObject(op);
}
}

[Test]
public void ErrorHandling()
{
bool called = false;
var errorMessage = "";
EventHandler<Finalizer.ErrorArgs> handleFunc = (sender, args) =>
{
called = true;
errorMessage = args.Error.Message;
};
Finalizer.Instance.Threshold = 1;
Finalizer.Instance.ErrorHandler += handleFunc;
try
{
WeakReference shortWeak;
WeakReference longWeak;
{
MakeAGarbage(out shortWeak, out longWeak);
var obj = (PyLong)longWeak.Target;
IntPtr handle = obj.Handle;
shortWeak = null;
longWeak = null;
MyPyObject.CreateMyPyObject(handle);
obj.Dispose();
obj = null;
}
FullGCCollect();
Finalizer.Instance.Collect();
Assert.IsTrue(called);
}
finally
{
Finalizer.Instance.ErrorHandler -= handleFunc;
}
Assert.AreEqual(errorMessage, "MyPyObject");
}

[Test]
public void ValidateRefCount()
{
Expand Down Expand Up @@ -279,36 +219,13 @@ public void ValidateRefCount()
}
}

[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)] // ensure lack of references to s1 and s2
private static IntPtr CreateStringGarbage()
{
PyString s1 = new PyString("test_string");
// s2 steal a reference from s1
PyString s2 = new PyString(s1.Handle);
return s1.Handle;
}

private static bool WaitForCollected(IntPtr op, int hash, int milliseconds)
{
var stopwatch = Stopwatch.StartNew();
do
{
var garbage = Finalizer.Instance.GetCollectedObjects();
foreach (var item in garbage)
{
// The validation is not 100% precise,
// but it's rare that two conditions satisfied but they're still not the same object.
if (item.Target.GetHashCode() != hash)
{
continue;
}
var obj = (IPyDisposable)item.Target;
if (obj.GetTrackedHandles().Contains(op))
{
return true;
}
}
} while (stopwatch.ElapsedMilliseconds < milliseconds);
return false;
}
}
}
7 changes: 7 additions & 0 deletions src/runtime/debughelper.cs
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -137,5 +137,12 @@ public static void PrintHexBytes(byte[] bytes)
Console.WriteLine();
}
}

[Conditional("DEBUG")]
public static void AssertHasReferences(IntPtr obj)
{
long refcount = Runtime.Refcount(obj);
Debug.Assert(refcount > 0, "Object refcount is 0 or less");
}
}
}
9 changes: 2 additions & 7 deletions src/runtime/delegatemanager.cs
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ A possible alternate strategy would be to create custom subclasses
too "special" for this to work. It would be more work, so for now
the 80/20 rule applies :) */

public class Dispatcher : IPyDisposable
public class Dispatcher
{
public IntPtr target;
public Type dtype;
Expand All @@ -202,7 +202,7 @@ public Dispatcher(IntPtr target, Type dtype)
return;
}
_finalized = true;
Finalizer.Instance.AddFinalizedObject(this);
Finalizer.Instance.AddFinalizedObject(ref target);
}

public void Dispose()
Expand Down Expand Up @@ -276,11 +276,6 @@ public object TrueDispatch(ArrayList args)
Runtime.XDecref(op);
return result;
}

public IntPtr[] GetTrackedHandles()
{
return new IntPtr[] { target };
}
}


Expand Down
Loading
Back | FazBrowse Home | New Git URL