[main] Update dependencies from dotnet/installer #6363

dotnet-maestro · 2021-10-04T12:07:23Z

This pull request updates the following dependencies

Coherency Updates

The following updates ensure that dependencies with a CoherentParentDependency
attribute were produced in a build used as input to the parent dependency's build.
See Dependency Description Format

Coherency Updates:
- Microsoft.NETCore.App.Ref: from 6.0.0-rtm.21472.13 to 6.0.0-rtm.21517.2 (parent: Microsoft.Dotnet.Sdk.Internal)

From https://github.com/dotnet/installer

Subscription: 6548876b-06a1-4ab6-a5a5-08d8ed868088
Build: 20211018.6
Date Produced: October 18, 2021 11:40:38 AM UTC
Commit: 4cba5995022e769f2f9688ce44d8fc204559e4d9
Branch: refs/heads/release/6.0.1xx

Updates:
- Microsoft.Dotnet.Sdk.Internal: from 6.0.100-rtm.21476.2 to 6.0.100-rtm.21518.6
- Microsoft.NETCore.App.Ref: from 6.0.0-rtm.21472.13 to 6.0.0-rtm.21517.2

…210930.21 Microsoft.Dotnet.Sdk.Internal From Version 6.0.100-rtm.21476.2 -> To Version 6.0.100-rtm.21480.21 Dependency coherency updates Microsoft.NETCore.App.Ref From Version 6.0.0-rtm.21472.13 -> To Version 6.0.0-rtm.21479.7 (parent: Microsoft.Dotnet.Sdk.Internal

jonpryor · 2021-10-07T01:49:59Z

The remaining failure is in Java.InteropTests.JavaObjectTest.Dispose_Finalized:

Expected: True
But was:  False

   at Java.InteropTests.JavaObjectTest.Dispose_Finalized()
   at System.Reflection.RuntimeMethodInfo.Invoke(Object , BindingFlags , Binder , Object[] , CultureInfo )

The test:

The failing assert is Assert.IsTrue (f). The test expects the JavaDisposedObject finalizer to be executed; when the finalizer is executed, JavaDisposedObject.Dispose(false) will be invoked, which will invoke JavaDisposedObject.OnFinalized(), which sets f to true. The assertion fails because f is false, presumably because JavaDisposedObject.OnFinalized() isn't invoked, presumably because .Dispose(false) isn't invoked, presumably because the finalizer isn't invoked.

Has the .NET 6 interpreter changed GC finalizer semantics?

dotnet/runtime changes: http://github.com/dotnet/runtime/compare/491ed9a112559872c31ae40da8f6d9977ba4ce3a...e013f14c5330375a18656dea8dca14e5602a3ae0

BrzVlad · 2021-10-07T07:27:18Z

@jonpryor Nothing changed. The failure is likely just because it is hard to guarantee object finalization with mono. The best way to deterministically finalize objects, that was proven to work fine, is https://github.com/mono/mono/blob/main/mono/mini/TestHelpers.cs#L12

jonpryor · 2021-10-08T17:23:42Z

@BrzVlad : then what are we doing "wrong"? https://github.com/xamarin/java.interop/blob/4277ac96eb4f2dd191e86f7bc1b72f7bdd4fbb0c/tests/Java.Interop-Tests/Java.Interop/JavaObjectTest.cs#L112-L129

We're allocating the "to-be-finalized" instance on its own Thread, and running GC.Collect(); GC.WaitForPendingFinalizers() twice, and the instance isn't being collected.

Do we need to add a new object() to the Thread to address:

				// 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.

BrzVlad · 2021-10-08T17:45:50Z

@jonpryor It's either that or that the object ref is not stored deep enough on the call stack. I recommend you just copy that method in some shared testing library / sources, and then you use FinalizerHelpers.PerformNoPinAction in all tests that require waiting for finalization. As we did in mono/mono.

…211008.19 Microsoft.Dotnet.Sdk.Internal From Version 6.0.100-rtm.21476.2 -> To Version 6.0.100-rtm.21508.19 Dependency coherency updates Microsoft.NETCore.App.Ref From Version 6.0.0-rtm.21472.13 -> To Version 6.0.0-rtm.21508.5 (parent: Microsoft.Dotnet.Sdk.Internal

jonathanpeppers · 2021-10-11T13:29:52Z

@BrzVlad this test also started failing on this bump for the private RC 2 builds:

https://github.com/xamarin/xamarin-android-private/pull/1

I believe this test has been passing for all .NET 6 development until now. Do you know why it started failing?

Context: dotnet/android#6363 We're seeing a test failure in the latest .NET 6 bump: Expected: True But was: False at Java.InteropTests.JavaObjectTest.Dispose_Finalized() at System.Reflection.RuntimeMethodInfo.Invoke(Object , BindingFlags , Binder , Object[] , CultureInfo ) The 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.

jonathanpeppers · 2021-10-15T15:38:55Z

Got sidetracked with the Hackathon, but I pushed a commit to see if FinalizerHelper fixes this.

jonpryor · 2021-10-16T00:00:37Z

Looks like it's still busted, even after adding that FinalizeHelper change to Java.Interop: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=5334115&view=ms.vss-test-web.build-test-results-tab&runId=27275994&resultId=100383&paneView=debug

I think "something" actually broke in this bump.

…211018.6 Microsoft.Dotnet.Sdk.Internal From Version 6.0.100-rtm.21476.2 -> To Version 6.0.100-rtm.21518.6 Dependency coherency updates Microsoft.NETCore.App.Ref From Version 6.0.0-rtm.21472.13 -> To Version 6.0.0-rtm.21517.2 (parent: Microsoft.Dotnet.Sdk.Internal

jonathanpeppers · 2021-10-18T16:31:52Z

@BrzVlad I could repro this with:

https://github.com/jonathanpeppers/net6-finalizer-repro

It is interesting that it worked fine with one call, and breaks when I used Parallel.For (0, 100, ...):

jonathanpeppers/net6-finalizer-repro@a2fd388

jonathanpeppers · 2021-10-18T19:35:07Z

Ok, I refine the repro some more.

It seems to happen with a for-loop, and you don't need to use the interpreter:

https://github.com/jonathanpeppers/net6-finalizer-repro

I added a UpdateMonoRuntimePacks you can downgrade the Mono runtime packs and see the issue goes away.

This reverts commit d91fe45.

) Context: dotnet/android#6363 Context: dotnet/android#6363 (comment) Context: #893 Context: dotnet/runtime#60638 Context: dotnet/runtime@491ed9a...e013f14 An "odd" problem arose in dotnet/android#6363, which attempted to bump to dotnet/runtime@e013f14c: the [`JavaObjectTest.Dispose_Finalized()` test][0] started failing. Doubly odd is that this test has been working fine for *years*, and there are no "obviously relevant" changes in the dotnet/runtime diff. There was a suggestion to use try using [`FinalizeHelpers.PerformNoPinAction()`][1], but this didn't fix the failing unit test. In order to "unblock" the dotnet/runtime bump in xamarin/xamarin-android, add a `[Category ("IgnoreInterpreter")]` to the `Dispose_Finalized()` test, so that it can be ignored for now. We have also filed dotnet/runtime#60638 to track a proper fix. [0]: https://github.com/xamarin/java.interop/blob/4277ac96eb4f2dd191e86f7bc1b72f7bdd4fbb0c/tests/Java.Interop-Tests/Java.Interop/JavaObjectTest.cs#L112-L129 [1]: https://github.com/mono/mono/blob/8266c5604b8c03882f2b06af27fdea46b142d6b9/mono/mini/TestHelpers.cs#L12-L44

This reverts commit b6d1a0a.

Changes: dotnet/java-interop@2d5431f...d1d64c1 [tests] add [Category("IgnoreInterpreter")] to Dispose_Finalized()

Context: dotnet/android#6363 We're seeing a test failure in the latest .NET 6 bump: Expected: True But was: False at Java.InteropTests.JavaObjectTest.Dispose_Finalized() at System.Reflection.RuntimeMethodInfo.Invoke(Object , BindingFlags , Binder , Object[] , CultureInfo ) The 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.

Context: dotnet/runtime#60638 (comment) Context: dotnet/android#6363 We were seeing `JavaObjectTest.Dispose_Finalized()` fail in a .NET 6 bump 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. This did not solve this issue; we need to fix up the test to wait for two GCs to complete: FinalizerHelpers.PerformNoPinAction (() => { FinalizerHelpers.PerformNoPinAction (() => { var v = new JavaDisposedObject (() => d = true, () => f = true); GC.KeepAlive (v); }); JniEnvironment.Runtime.ValueManager.CollectPeers (); }); JniEnvironment.Runtime.ValueManager.CollectPeers (); With this change in place, the test now passes: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=5361266&view=ms.vss-test-web.build-test-results-tab We can also remove the category added in d1d64c1.

@BrzVlad

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.

dotnet-maestro bot requested review from jonathanpeppers, jonpryor and pjcollins as code owners October 4, 2021 12:07

jonathanpeppers mentioned this pull request Oct 15, 2021

[tests] use FinalizerHelper from mono/mono dotnet/java-interop#893

Closed

jonathanpeppers marked this pull request as draft October 15, 2021 15:38

jonathanpeppers added 2 commits October 15, 2021 10:44

Merge branch 'main' into darc-main-14333f32-1f73-4599-8639-ac5d3fcbe79d

18bcd74

Testing dotnet/java-interop#893

d91fe45

jonathanpeppers force-pushed the darc-main-14333f32-1f73-4599-8639-ac5d3fcbe79d branch from 15585ab to d91fe45 Compare October 15, 2021 15:44

jonathanpeppers mentioned this pull request Oct 19, 2021

[android] interpreter does not run Java/C# object finalizers in some cases dotnet/runtime#60638

Closed

jonathanpeppers added 4 commits October 19, 2021 12:22

Revert "Testing dotnet/java-interop#893"

94461c9

This reverts commit d91fe45.

Merge branch 'main' into darc-main-14333f32-1f73-4599-8639-ac5d3fcbe79d

71b7d3b

Testing: dotnet/java-interop#895

b6d1a0a

Set IgnoreInterpreter category for UseInterpreter=true

13fa9e9

jonathanpeppers mentioned this pull request Oct 19, 2021

[tests] add IgnoreInterpreter category for one test dotnet/java-interop#895

Merged

jonathanpeppers added 2 commits October 20, 2021 14:35

Revert "Testing: dotnet/java-interop#895"

3674016

This reverts commit b6d1a0a.

Merge branch 'main' into darc-main-14333f32-1f73-4599-8639-ac5d3fcbe79d

f50301f

Bump to xamarin/java.interop/main@d1d64c10

bc57246

Changes: dotnet/java-interop@2d5431f...d1d64c1 [tests] add [Category("IgnoreInterpreter")] to Dispose_Finalized()

jonathanpeppers marked this pull request as ready for review October 20, 2021 20:00

jonpryor merged commit c2c9ed4 into main Oct 20, 2021

jonpryor deleted the darc-main-14333f32-1f73-4599-8639-ac5d3fcbe79d branch October 20, 2021 21:57

jonathanpeppers mentioned this pull request Oct 22, 2021

[tests] rework JavaObjectTest, use FinalizerHelper from mono/mono dotnet/java-interop#899

Merged

github-actions bot locked and limited conversation to collaborators Jan 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[main] Update dependencies from dotnet/installer #6363

[main] Update dependencies from dotnet/installer #6363

Uh oh!

dotnet-maestro bot commented Oct 4, 2021 •

edited

Loading

Uh oh!

jonpryor commented Oct 7, 2021

Uh oh!

BrzVlad commented Oct 7, 2021

Uh oh!

jonpryor commented Oct 8, 2021

Uh oh!

BrzVlad commented Oct 8, 2021

Uh oh!

jonathanpeppers commented Oct 11, 2021

Uh oh!

jonathanpeppers commented Oct 15, 2021

Uh oh!

jonpryor commented Oct 16, 2021 •

edited

Loading

Uh oh!

jonathanpeppers commented Oct 18, 2021 •

edited

Loading

Uh oh!

jonathanpeppers commented Oct 18, 2021

Uh oh!

Uh oh!

[main] Update dependencies from dotnet/installer #6363

[main] Update dependencies from dotnet/installer #6363

Uh oh!

Conversation

dotnet-maestro bot commented Oct 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coherency Updates

From https://github.com/dotnet/installer

Uh oh!

jonpryor commented Oct 7, 2021

Uh oh!

BrzVlad commented Oct 7, 2021

Uh oh!

jonpryor commented Oct 8, 2021

Uh oh!

BrzVlad commented Oct 8, 2021

Uh oh!

jonathanpeppers commented Oct 11, 2021

Uh oh!

jonathanpeppers commented Oct 15, 2021

Uh oh!

jonpryor commented Oct 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonathanpeppers commented Oct 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonathanpeppers commented Oct 18, 2021

Uh oh!

Uh oh!

dotnet-maestro bot commented Oct 4, 2021 •

edited

Loading

jonpryor commented Oct 16, 2021 •

edited

Loading

jonathanpeppers commented Oct 18, 2021 •

edited

Loading