Skip to content

Commit

Permalink
Avoid multiple java/lang/Object bindings.
Browse files Browse the repository at this point in the history
A funny thing happened when b9aa5f7 ran on xamarin-android:
unit tests started crashing!

	E monodroid-assembly: typemap: unable to load assembly 'Java.Interop-Tests' when looking up managed type corresponding to Java type 'java/lang/Object'

What appears to be happening is an Unfortunate Interaction™:

 1. `Java.Interop-Tests.dll` contained *multiple bindings* for
    `java/lang/Object`. e.g.

        [JniTypeSignature ("java/lang/Object", GenerateJavaPeer=false)]
        partial class JavaDisposedObject : JavaObject {
        }

 2. The typemap generator has no functionality to "prioritize" one
    binding vs. another; it's random.  As such, there is nothing to
    cause `Java.Lang.Object` to be used as the preferred binding for
    `java/lang/Object`.

This meant that when we hit the typemap codepath in .NET Android,
we looked for the C# type that corresponded to `java/lang/Object`,
found *some random type* from `Java.Interop-Tests`, and…

…and then we hit another oddity: that codepath only supported looking
for C# types in assemblies which had already been loaded.  This was
occurring during startup, so `Java.Interop-Tests` had not yet been
loaded yet, so it errored out, returned `nullptr`, and later Android
just aborts things:

	F droid.NET_Test: runtime.cc:638] JNI DETECTED ERROR IN APPLICATION: use of deleted local reference 0x79

Just…eep!

This didn't happen before because `Java.Interop.JavaObject` subclasses
*didn't* participate in typemap generation.  b9aa5f7 *added* that
support, introducing this unforeseen interaction.

Fix this by *removing* all "alternate bindings" for `java/lang/Object`:

	- [JniTypeSignature ("java/lang/Object", GenerateJavaPeer=false)]
	+ [JniTypeSignature (JniTypeName)]
          partial class JavaDisposedObject : JavaObject {
              internal const string JniTypeName = "net/dot/jni/test/JavaDisposedObject";
          }

This implicitly requires that we now have a Java Callable Wrapper
for this type, so update `Java.Interop-Tests.csproj` to run `jcw-gen`
as part of the build process.  This ensures that we create the
JCW for e.g. `JavaDisposedObject`.

Finally, update `JavaVMFixture` to add the required typemap entries.

These changes should allow .NET Android unit tests to run w/o crashing.
  • Loading branch information
jonpryor committed Jan 25, 2024
1 parent b9aa5f7 commit 6b3637d
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 6 deletions.
39 changes: 37 additions & 2 deletions tests/Java.Interop-Tests/Java.Interop-Tests.targets
Original file line number Diff line number Diff line change
@@ -1,12 +1,47 @@
<Project>

<ItemGroup>
<_BuildJavaInteropTestsJarInputs Include="$(TargetPath)" />
<_BuildJavaInteropTestsJarInputs Include="$(MSBuildThisFileFullPath)" />
<_BuildJavaInteropTestsJarInputs Include="java\**\*.java" />
</ItemGroup>

<Target Name="_CreateJavaCallableWrappers"
Condition=" '$(TargetPath)' != '' "
AfterTargets="Build"
Inputs="@(_BuildJavaInteropTestsJarInputs)"
Outputs="$(IntermediateOutputPath)java\.stamp">
<RemoveDir Directories="$(IntermediateOutputPath)java" />
<MakeDir Directories="$(IntermediateOutputPath)java" />
<ItemGroup>
<!-- I can't find a good way to trim the trailing `\`, so append with `.` so we can sanely quote for $(_Libpath) -->
<_RefAsmDirs Include="@(ReferencePathWithRefAssemblies->'%(RootDir)%(Directory).'->Distinct())" />
</ItemGroup>
<PropertyGroup>
<_JcwGen>"$(UtilityOutputFullPath)/jcw-gen.dll"</_JcwGen>
<_Target>--codegen-target JavaInterop1</_Target>
<_Output>-o "$(IntermediateOutputPath)/java"</_Output>
<_Libpath>@(_RefAsmDirs->'-L "%(Identity)"', ' ')</_Libpath>
</PropertyGroup>
<Exec Command="$(DotnetToolPath) $(_JcwGen) -v &quot;$(TargetPath)&quot; $(_Target) $(_Output) $(_Libpath)" />
<Touch Files="$(IntermediateOutputPath)java\.stamp" AlwaysCreate="True" />
</Target>

<Target Name="_CollectGeneratdJcwSource">
<ItemGroup>
<_GeneratedJcwSource Include="$(IntermediateOutputPath)java\**\*.java" />
</ItemGroup>
</Target>

<Target Name="BuildInteropTestJar"
BeforeTargets="Build"
Inputs="@(JavaInteropTestJar)"
AfterTargets="Build"
DependsOnTargets="_CreateJavaCallableWrappers;_CollectGeneratdJcwSource"
Inputs="@(JavaInteropTestJar);@(_GeneratedJcwSource)"
Outputs="$(OutputPath)interop-test.jar">
<MakeDir Directories="$(IntermediateOutputPath)it-classes" />
<ItemGroup>
<_Source Include="@(JavaInteropTestJar->Replace('%5c', '/'))" />
<_Source Include="@(_GeneratedJcwSource->Replace('%5c', '/'))" />
</ItemGroup>
<WriteLinesToFile
File="$(IntermediateOutputPath)_java_sources.txt"
Expand Down
16 changes: 12 additions & 4 deletions tests/Java.Interop-Tests/Java.Interop/JavaObjectTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,13 @@ public void Ctor_Exceptions ()
var r = new JniObjectReference ();
Assert.Throws<ArgumentException> (() => new JavaObject (ref r, JniObjectReferenceOptions.CopyAndDispose));

// Note: This may break if/when JavaVM provides "default"
Assert.Throws<NotSupportedException> (() => new JavaObjectWithNoJavaPeer ());
#if __ANDROID__
Assert.Throws<Java.Lang.ClassNotFoundException> (() => new JavaObjectWithMissingJavaPeer ()).Dispose ();
#else // !__ANDROID__
// Note: `JavaObjectWithNoJavaPeer` creation works on Android because tooling provides all
// typemap entries. On desktop, we use the hardcoded cit in JavaVMFixture, which deliberately
// *lacks* an entry for `JavaObjectWithNoJavaPeer`.
Assert.Throws<NotSupportedException> (() => new JavaObjectWithNoJavaPeer ());
Assert.Throws<JavaException> (() => new JavaObjectWithMissingJavaPeer ()).Dispose ();
#endif // !__ANDROID__
}
Expand Down Expand Up @@ -201,17 +203,21 @@ public void DisposeAccessesThis ()
}
}

#if !__ANDROID__
class JavaObjectWithNoJavaPeer : JavaObject {
}
#endif // !__ANDROID__

[JniTypeSignature (JniTypeName, GenerateJavaPeer=false)]
class JavaObjectWithMissingJavaPeer : JavaObject {
internal const string JniTypeName = "__this__/__type__/__had__/__better__/__not__/__Exist__";
}

[JniTypeSignature ("java/lang/Object", GenerateJavaPeer=false)]
[JniTypeSignature (JniTypeName)]
class JavaDisposedObject : JavaObject {

internal const string JniTypeName = "net/dot/jni/test/JavaDisposedObject";

public Action OnDisposed;
public Action OnFinalized;

Expand All @@ -230,8 +236,10 @@ protected override void Dispose (bool disposing)
}
}

[JniTypeSignature ("java/lang/Object", GenerateJavaPeer=false)]
[JniTypeSignature (JniTypeName)]
class MyDisposableObject : JavaObject {
internal const string JniTypeName = "net/dot/jni/test/MyDisposableObject";

bool _isDisposed;

public MyDisposableObject ()
Expand Down
3 changes: 3 additions & 0 deletions tests/Java.Interop-Tests/Java.Interop/JavaVMFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ class JavaVMFixtureTypeManager : JniRuntime.JniTypeManager {
[CallVirtualFromConstructorBase.JniTypeName] = typeof (CallVirtualFromConstructorBase),
[CallVirtualFromConstructorDerived.JniTypeName] = typeof (CallVirtualFromConstructorDerived),
[GetThis.JniTypeName] = typeof (GetThis),
[JavaDisposedObject.JniTypeName] = typeof (JavaDisposedObject),
[JavaObjectWithMissingJavaPeer.JniTypeName] = typeof (JavaObjectWithMissingJavaPeer),
[MyDisposableObject.JniTypeName] = typeof (JavaDisposedObject),
};

public JavaVMFixtureTypeManager ()
Expand Down

0 comments on commit 6b3637d

Please sign in to comment.