[Export] Fix dynamic export array callbacks

[simonrozsival]

Stacked on #11123
Closes #1259

This PR keeps the trimmable typemap export PR focused on the new trimmable path, and moves the unrelated legacy Mono.Android.Export dynamic callback fixes here. The end result is that the new array [Export] tests can run head-to-head on the legacy non-trimmable typemap paths too, instead of being disabled there.

Where this broke

I traced the JNIEnv.GetArray<T> and Mono.Android.Export history. This was not a later JNIEnv.GetArray<T> signature change that regressed working code. The mismatch was present from the original Mono.Android.Export import in d92a49d56 ([Mono.Android.Export] Import from monodroid/d393f359, 2016-04-21):

• CallbackCode.cs cached jnienv_getarray from JNIEnv.GetArray<int>, whose signature was already GetArray<T>(IntPtr array_ptr).
• The array callback prep path nevertheless emitted a call with three arguments: (IntPtr, JniHandleOwnership.DoNotTransfer, Type).

So array-shaped dynamic [Export] callbacks appear to have been broken since that import. The new tests in #11123 are what exposed the bug because they exercise [Export] methods with array parameters/returns on the legacy dynamic-export path.

What used to crash

A small app/test like this is enough to trigger the broken legacy dynamic callback generation:

class ExportIntArray : Java.Lang.Object
{
    [Export]
    public int [] DoubleArray (int [] xs)
    {
        for (int i = 0; i < xs.Length; i++) {
            xs [i] *= 2;
        }
        return xs;
    }
}

When Java/JNI calls DoubleArray(int[]), Mono.Android.Export dynamically emits a native callback delegate that converts the incoming jintArray to int[], calls the managed method, converts the return value back to JNI, and copies any managed array parameter mutations back to the original JNI array.

Before this fix, the generated callback was wrong in several ways. Simplified, it behaved like this:

// Parameter prep: CallbackCode cached JNIEnv.GetArray<T>(IntPtr), but emitted three arguments.
ldarg.2                              // jintArray parameter
ldc.i4.0                             // JniHandleOwnership.DoNotTransfer
ldtoken int32[]
call Android.Runtime.JNIEnv::GetArray<int32>(native int, valuetype JniHandleOwnership, class System.Type)
castclass int32[]
call instance int32[] ExportIntArray::DoubleArray(int32[])
call native int Android.Runtime.JNIEnv::NewArray<int32>(int32[])

// Cleanup/copy-back: used the wrong JNI argument slot and was represented as an invalid ternary
// instead of an if statement.
ldarg.0                              // BUG: this is JNIEnv*, not the original jintArray parameter
call void Android.Runtime.JNIEnv::CopyArray(int32[], native int)

Depending on exactly how far callback generation got, this could fail while creating the dynamic method (for example invalid GetArray/CopyArray calls or an invalid ternary expression mixing int[] and void) or later abort under CheckJNI because registration left a pending managed exception.

With the fix, the generated shape is instead:

// Parameter prep: call the one-argument JNIEnv.GetArray<T>(IntPtr) method and materialize the managed array.
ldarg.2                              // jintArray parameter
call int32[] Android.Runtime.JNIEnv::GetArray<int32>(native int)
stloc.s managedArray

// Call the user's exported method with the same managed array instance that cleanup will copy back.
ldloc.s managedArray
call instance int32[] ExportIntArray::DoubleArray(int32[])
call native int Android.Runtime.JNIEnv::NewArray<int32>(int32[])
stloc.s abiReturn

// Cleanup/copy-back: if (managedArray != null) JNIEnv.CopyArray(managedArray, originalJniArray);
ldloc.s managedArray
brfalse.s doneCopyBack
ldloc.s managedArray
ldarg.2                              // original jintArray parameter
call void Android.Runtime.JNIEnv::CopyArray(int32[], native int)
doneCopyBack:
ldloc.s abiReturn
ret

That fixes both the crash and the semantic bug: mutations to the managed int[] parameter are copied back to the original JNI array handle.

Changes

• Fix array callback prep to call the one-argument JNIEnv.GetArray<T>(IntPtr) method that CallbackCode already cached.
• Materialize prepared array parameters so copy-back updates the managed instance passed to the exported method.
• Copy array parameters back to the correct JNI argument slot.
• Use the array element type when selecting JNIEnv.CopyArray overloads.
• Emit array cleanup as an if statement instead of an invalid ternary expression.
• Re-enable the int-array and peer-array export tests on non-trimmable typemap paths.

Validation

Validation performed locally before splitting this out from #11123:

• CoreCLR llvm-ir IncludeCategories=Export passed with scalar/object/array tests enabled and exception-routing ignored.
• Mono llvm-ir IncludeCategories=Export passed with scalar/object/array tests enabled and exception-routing ignored.
• CoreCLR trimmable IncludeCategories=Export passed all export method tests.

[Copilot]

Pull request overview

This PR updates the legacy Mono.Android.Export dynamic callback generator to correctly marshal [Export] array parameters (including copy-back semantics), and re-enables the previously skipped array export tests so non-trimmable typemap paths are covered again.

Changes:

• Adjust callback array preparation to use JNIEnv.GetArray<T>(IntPtr) and ensure prepared array parameters are materialized so copy-back updates the same managed instance passed to the exported method.
• Fix copy-back to target the correct JNI argument slot and choose JNIEnv.CopyArray overloads based on array element type.
• Re-enable int-array and peer-array [Export] device tests by removing the trimmable-only ignore gate.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/ExportTests.cs	Removes the trimmable-only skip so array export tests run again on legacy export paths.
src/Mono.Android.Export/CallbackCode.cs	Fixes legacy dynamic callback array marshalling and copy-back codegen in Mono.Android.Export.

Comments suppressed due to low confidence (1)

src/Mono.Android.Export/CallbackCode.cs:276

• GetCallbackCleanup()'s CharSequence case emits a Java.Lang.Object.GetObject<ICharSequence>(...) call during cleanup, but at the call site arg is the prepared managed ICharSequence value (not the original IntPtr). This will generate invalid IL / runtime failures for exported methods that take ICharSequence parameters. Cleanup should be a no-op for CharSequence parameters (e.g., return arg/empty statement) rather than re-wrapping the managed object.

			// CharSequenceSymbol:
			// 	return new string[] { String.Format ("Java.Lang.ICharSequence {0} = Java.Lang.Object.GetObject<Java.Lang.ICharSequence> ({1}, JniHandleOwnership.DoNotTransfer);", var_name, SymbolTable.GetNativeName (var_name)) };
			case SymbolKind.CharSequence:
				return new CodeMethodCall (object_getobject.MakeGenericMethod (typeof (ICharSequence)), arg, do_not_transfer_literal);

[simonrozsival]

/review

[github-actions]

✅ Android PR Reviewer completed successfully!

[github-actions]

✅ LGTM — well-analyzed long-standing bug fix

Excellent detective work tracing this back to the original 2016 import. The fix is correct across all four dimensions:

1. Prep: Calls the 1-arg generic GetArray<T>(IntPtr) that jnienv_getarray actually caches — fixes the argument count mismatch.
2. Materialization: DeclareVariable ensures the same managed array instance is passed to the exported method and used for copy-back.
3. Cleanup slot: mgen.GetArg(i + 2) correctly skips JNIEnv* and jobject to reach the original JNI array parameter.
4. Control flow: CodeIf instead of CodeWhen avoids the invalid ternary that mixed int[] and void return types.

The CopyArray overload resolution fix (elementType instead of type, IJavaObject[] instead of IJavaObject) and the null guard on copyArrayMethod are good catches too.

Re-enabling the two array export tests on the legacy path is the right call now that the underlying code gen is fixed.

CI

• ✅ license/cla passed
• ✅ dotnet-android passed
• ⏳ Xamarin.Android-PR — not visible in checks (may need trigger or re-run)

Generated by Android PR Reviewer for issue #11428 · ● 19.1M

[github-actions]

🤖 💡 Code organization — callbackCleanup is declared but never used (pre-existing). Now that this area is being touched, consider removing it to reduce noise.

Rule: Remove unused code