-
Notifications
You must be signed in to change notification settings - Fork 64
[tests] rework JavaObjectTest, use FinalizerHelper from mono/mono #899
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| // Originally from: https://github.com/mono/mono/blob/8266c5604b8c03882f2b06af27fdea46b142d6b9/mono/mini/TestHelpers.cs#L12 | ||
|
|
||
| using System; | ||
| using System.Threading; | ||
|
|
||
| namespace Java.InteropTests | ||
| { | ||
| // False pinning cases are still possible. For example the thread can die | ||
| // and its stack reused by another thread. It also seems that a thread that | ||
| // does a GC can keep on the stack references to objects it encountered | ||
| // during the collection which are never released afterwards. This would | ||
| // be more likely to happen with the interpreter which reuses more stack. | ||
| static class FinalizerHelpers | ||
| { | ||
| private static IntPtr aptr; | ||
|
|
||
| private static unsafe void NoPinActionHelper (int depth, Action act) | ||
| { | ||
| // Avoid tail calls | ||
| int* values = stackalloc int [20]; | ||
| aptr = new IntPtr (values); | ||
|
|
||
| if (depth <= 0) { | ||
| // | ||
| // When the action is called, this new thread might have not allocated | ||
| // anything yet in the nursery. This means that the address of the first | ||
| // object that would be allocated would be at the start of the tlab and | ||
| // implicitly the end of the previous tlab (address which can be in use | ||
| // when allocating on another thread, at checking if an object fits in | ||
| // this other tlab). We allocate a new dummy object to avoid this type | ||
| // of false pinning for most common cases. | ||
| // | ||
| new object (); | ||
| act (); | ||
| } else { | ||
| NoPinActionHelper (depth - 1, act); | ||
| } | ||
| } | ||
|
|
||
| public static void PerformNoPinAction (Action act) | ||
| { | ||
| Thread thr = new Thread (() => NoPinActionHelper (128, act)); | ||
| thr.Start (); | ||
| thr.Join (); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,4 @@ | ||
| using System; | ||
| using System.Threading; | ||
|
|
||
| using Java.Interop; | ||
|
|
||
|
|
@@ -18,13 +17,11 @@ public void JavaReferencedInstanceSurvivesCollection () | |
| using (var t = new JniType ("java/lang/Object")) { | ||
| var oldHandle = IntPtr.Zero; | ||
| var array = new JavaObjectArray<JavaObject> (1); | ||
| var w = new Thread (() => { | ||
| FinalizerHelpers.PerformNoPinAction (() => { | ||
| var v = new JavaObject (); | ||
| oldHandle = v.PeerReference.Handle; | ||
| array [0] = v; | ||
| }); | ||
| w.Start (); | ||
| w.Join (); | ||
| JniEnvironment.Runtime.ValueManager.CollectPeers (); | ||
| GC.WaitForPendingFinalizers (); | ||
| GC.WaitForPendingFinalizers (); | ||
|
|
@@ -80,13 +77,11 @@ public void UnreferencedInstanceIsCollected () | |
| { | ||
| JniObjectReference oldHandle = new JniObjectReference (); | ||
| WeakReference r = null; | ||
| var t = new Thread (() => { | ||
| FinalizerHelpers.PerformNoPinAction (() => { | ||
| var v = new JavaObject (); | ||
| oldHandle = v.PeerReference.NewWeakGlobalRef (); | ||
| r = new WeakReference (v); | ||
| }); | ||
| t.Start (); | ||
| t.Join (); | ||
| JniEnvironment.Runtime.ValueManager.CollectPeers (); | ||
| GC.WaitForPendingFinalizers (); | ||
| GC.WaitForPendingFinalizers (); | ||
|
|
@@ -110,20 +105,17 @@ public void Dispose () | |
|
|
||
| #if !NO_GC_BRIDGE_SUPPORT | ||
| [Test] | ||
| // See: https://github.com/dotnet/runtime/issues/60638 | ||
| [Category ("IgnoreInterpreter")] | ||
| public void Dispose_Finalized () | ||
| { | ||
| var d = false; | ||
| var f = false; | ||
| var t = new Thread (() => { | ||
| var v = new JavaDisposedObject (() => d = true, () => f = true); | ||
| GC.KeepAlive (v); | ||
| FinalizerHelpers.PerformNoPinAction (() => { | ||
| FinalizerHelpers.PerformNoPinAction (() => { | ||
| var v = new JavaDisposedObject (() => d = true, () => f = true); | ||
| GC.KeepAlive (v); | ||
| }); | ||
| JniEnvironment.Runtime.ValueManager.CollectPeers (); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change somewhat confuses me: we need to call This is very confusing to me.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From what I understood a single call to And so we need two GCs for the finalizer to always run in this case. @BrzVlad is that correct? Details here: dotnet/runtime#60638 (comment)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem is that, 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 |
||
| }); | ||
| t.Start (); | ||
| t.Join (); | ||
| JniEnvironment.Runtime.ValueManager.CollectPeers (); | ||
| GC.WaitForPendingFinalizers (); | ||
| JniEnvironment.Runtime.ValueManager.CollectPeers (); | ||
| GC.WaitForPendingFinalizers (); | ||
| Assert.IsFalse (d); | ||
|
|
@@ -185,11 +177,9 @@ public void Ctor_Exceptions () | |
| public void CrossThreadSharingRequresRegistration () | ||
| { | ||
| JavaObject o = null; | ||
| var t = new Thread (() => { | ||
| FinalizerHelpers.PerformNoPinAction (() => { | ||
| o = new JavaObject (); | ||
| }); | ||
| t.Start (); | ||
| t.Join (); | ||
| o.ToString (); | ||
| o.Dispose (); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BrzVlad: Why is this member needed at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Answer: probably to prevent optimizing out the
stackalloc: https://discord.com/channels/732297728826277939/732297837953679412/902261731471007765