Skip to content

Commit 220b87f

Browse files
[tests] rework JavaObjectTest, use FinalizerHelper from mono/mono (#899)
Context: dotnet/android#6363 Context: dotnet/runtime#60638 (comment) Context: d1d64c1 Context: dotnet/android@c2c9ed4 We were seeing `JavaObjectTest.Dispose_Finalized()` fail in a .NET 6 bump in xamarin-android with `$(UseInterpreter)` set to `true`: Expected: True But was: False at Java.InteropTests.JavaObjectTest.Dispose_Finalized() at System.Reflection.RuntimeMethodInfo.Invoke(Object , BindingFlags , Binder , Object[] , CultureInfo ) The first recommendation was to use a helper method from mono/mono's unit tests: * https://github.com/mono/mono/blob/8266c5604b8c03882f2b06af27fdea46b142d6b9/mono/mini/TestHelpers.cs#L12 I removed usage of all `System.Threading` in `JavaObjectTest` in favor of this helper method, which internally uses a `Thread`. This did not solve this issue; we need to fix up the test to wait for two GCs to complete, on two separate threads: FinalizerHelpers.PerformNoPinAction (() => { FinalizerHelpers.PerformNoPinAction (() => { var v = new JavaDisposedObject (() => d = true, () => f = true); GC.KeepAlive (v); }); // Thread 2 JniEnvironment.Runtime.ValueManager.CollectPeers (); }); // Thread 1 JniEnvironment.Runtime.ValueManager.CollectPeers (); `ValueManager.CollectPeers()` uses `GC.Collect()`, and `GC.Collect()` needs to be called from two separate threads because, as per @BrzVlad: > if an object is alive then the GC will scan it and, by doing so, it > will probably store it in the stack somewhere. In very unfortunate > scenarios, at the second collection, the pinning step will find this > reference existing on the stack and will pin the object (that might > have now been dead otherwise). Because the `JavaDisposedObject` is > alive during the first collection, we do this collection on a > separate thread, so the second GC doesn't get to find this left over > references on the stack. With this change in place, the test now passes. We can also remove the category added in d1d64c1.
1 parent f658ab2 commit 220b87f

File tree

2 files changed

+56
-19
lines changed

2 files changed

+56
-19
lines changed
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Originally from: https://github.com/mono/mono/blob/8266c5604b8c03882f2b06af27fdea46b142d6b9/mono/mini/TestHelpers.cs#L12
2+
3+
using System;
4+
using System.Threading;
5+
6+
namespace Java.InteropTests
7+
{
8+
// False pinning cases are still possible. For example the thread can die
9+
// and its stack reused by another thread. It also seems that a thread that
10+
// does a GC can keep on the stack references to objects it encountered
11+
// during the collection which are never released afterwards. This would
12+
// be more likely to happen with the interpreter which reuses more stack.
13+
static class FinalizerHelpers
14+
{
15+
private static IntPtr aptr;
16+
17+
private static unsafe void NoPinActionHelper (int depth, Action act)
18+
{
19+
// Avoid tail calls
20+
int* values = stackalloc int [20];
21+
aptr = new IntPtr (values);
22+
23+
if (depth <= 0) {
24+
//
25+
// When the action is called, this new thread might have not allocated
26+
// anything yet in the nursery. This means that the address of the first
27+
// object that would be allocated would be at the start of the tlab and
28+
// implicitly the end of the previous tlab (address which can be in use
29+
// when allocating on another thread, at checking if an object fits in
30+
// this other tlab). We allocate a new dummy object to avoid this type
31+
// of false pinning for most common cases.
32+
//
33+
new object ();
34+
act ();
35+
} else {
36+
NoPinActionHelper (depth - 1, act);
37+
}
38+
}
39+
40+
public static void PerformNoPinAction (Action act)
41+
{
42+
Thread thr = new Thread (() => NoPinActionHelper (128, act));
43+
thr.Start ();
44+
thr.Join ();
45+
}
46+
}
47+
}

tests/Java.Interop-Tests/Java.Interop/JavaObjectTest.cs

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
using System;
2-
using System.Threading;
32

43
using Java.Interop;
54

@@ -18,13 +17,11 @@ public void JavaReferencedInstanceSurvivesCollection ()
1817
using (var t = new JniType ("java/lang/Object")) {
1918
var oldHandle = IntPtr.Zero;
2019
var array = new JavaObjectArray<JavaObject> (1);
21-
var w = new Thread (() => {
20+
FinalizerHelpers.PerformNoPinAction (() => {
2221
var v = new JavaObject ();
2322
oldHandle = v.PeerReference.Handle;
2423
array [0] = v;
2524
});
26-
w.Start ();
27-
w.Join ();
2825
JniEnvironment.Runtime.ValueManager.CollectPeers ();
2926
GC.WaitForPendingFinalizers ();
3027
GC.WaitForPendingFinalizers ();
@@ -80,13 +77,11 @@ public void UnreferencedInstanceIsCollected ()
8077
{
8178
JniObjectReference oldHandle = new JniObjectReference ();
8279
WeakReference r = null;
83-
var t = new Thread (() => {
80+
FinalizerHelpers.PerformNoPinAction (() => {
8481
var v = new JavaObject ();
8582
oldHandle = v.PeerReference.NewWeakGlobalRef ();
8683
r = new WeakReference (v);
8784
});
88-
t.Start ();
89-
t.Join ();
9085
JniEnvironment.Runtime.ValueManager.CollectPeers ();
9186
GC.WaitForPendingFinalizers ();
9287
GC.WaitForPendingFinalizers ();
@@ -110,20 +105,17 @@ public void Dispose ()
110105

111106
#if !NO_GC_BRIDGE_SUPPORT
112107
[Test]
113-
// See: https://github.com/dotnet/runtime/issues/60638
114-
[Category ("IgnoreInterpreter")]
115108
public void Dispose_Finalized ()
116109
{
117110
var d = false;
118111
var f = false;
119-
var t = new Thread (() => {
120-
var v = new JavaDisposedObject (() => d = true, () => f = true);
121-
GC.KeepAlive (v);
112+
FinalizerHelpers.PerformNoPinAction (() => {
113+
FinalizerHelpers.PerformNoPinAction (() => {
114+
var v = new JavaDisposedObject (() => d = true, () => f = true);
115+
GC.KeepAlive (v);
116+
});
117+
JniEnvironment.Runtime.ValueManager.CollectPeers ();
122118
});
123-
t.Start ();
124-
t.Join ();
125-
JniEnvironment.Runtime.ValueManager.CollectPeers ();
126-
GC.WaitForPendingFinalizers ();
127119
JniEnvironment.Runtime.ValueManager.CollectPeers ();
128120
GC.WaitForPendingFinalizers ();
129121
Assert.IsFalse (d);
@@ -185,11 +177,9 @@ public void Ctor_Exceptions ()
185177
public void CrossThreadSharingRequresRegistration ()
186178
{
187179
JavaObject o = null;
188-
var t = new Thread (() => {
180+
FinalizerHelpers.PerformNoPinAction (() => {
189181
o = new JavaObject ();
190182
});
191-
t.Start ();
192-
t.Join ();
193183
o.ToString ();
194184
o.Dispose ();
195185
}

0 commit comments

Comments
 (0)