Skip to content
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

Avoid non-blittable types in native callback methods #1027

Open
grendello opened this issue Aug 16, 2022 · 0 comments
Open

Avoid non-blittable types in native callback methods #1027

grendello opened this issue Aug 16, 2022 · 0 comments
Labels
enhancement Proposed change to current functionality generator Issues binding a Java library (generator, class-parse, etc.)
Milestone

Comments

@grendello
Copy link
Member

grendello commented Aug 16, 2022

While working on implementation of marshal methods (a way to replace the current native JNI method registration with
generated code) which take advantage of the [UnmanagedCallersOnly] attribute, I came across a problem that some of
our registered methods either return a bool or take a parameter which is a bool. The problem here is that bool
is a non-blittable type, while all [UnmanagedCallersOnly] methods must use only blittable types (as they are
called directly by the native code, bypassing marshaling).

Marshal methods convert the code output by the generator from:

  static Delegate? cb_setChildrenDrawingOrderEnabled_Z;
  static Delegate GetSetChildrenDrawingOrderEnabled_ZHandler ()
  {
    if (cb_setChildrenDrawingOrderEnabled_Z == null)
      cb_setChildrenDrawingOrderEnabled_Z = JNINativeWrapper.CreateDelegate ((_JniMarshal_PPZ_V) n_SetChildrenDrawingOrderEnabled_Z);
    return cb_setChildrenDrawingOrderEnabled_Z;
  }

  static void n_SetChildrenDrawingOrderEnabled_Z (IntPtr jnienv, IntPtr native__this, bool enabled)
  {
    var __this = global::Java.Lang.Object.GetObject<Android.Views.ViewGroup> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
    __this.ChildrenDrawingOrderEnabled = enabled;
  }

  protected virtual unsafe bool ChildrenDrawingOrderEnabled {
    [Register ("setChildrenDrawingOrderEnabled", "(Z)V", "GetSetChildrenDrawingOrderEnabled_ZHandler")]
    set {
      // ...
    }
  }

to

  [UnmanagedCallersOnly]
  static void n_SetChildrenDrawingOrderEnabled_Z (IntPtr jnienv, IntPtr native__this, bool enabled)
  {
    var __this = global::Java.Lang.Object.GetObject<Android.Views.ViewGroup> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
    __this.ChildrenDrawingOrderEnabled = enabled;
  }

  protected virtual unsafe bool ChildrenDrawingOrderEnabled {
    [Register ("setChildrenDrawingOrderEnabled", "(Z)V", "GetSetChildrenDrawingOrderEnabled_ZHandler")]
    set {
      // ...
    }
  }

And at run time, a pointer to n_SetChildrenDrawingOrderEnabled_Z is obtained using the Mono embedding API mono_method_get_unmanaged_callers_only_ftnptr.
However, when a non-blittable type is encountered, the function will return an error:

method Android.Views.ViewGroup:n_SetChildrenDrawingOrderEnabled_Z(intptr,intptr,bool) with UnmanagedCallersOnlyAttribute has non-blittable parameters or return type assembly:<unknown assembly> type:<unknown ty
pe> member:(null)

The problem is quite common in MAUI apps, since in a simple Hello World app, out of 183 marshal method candidates, 133 have a bool in their parameter list or as a return value.
I have implemented a workaround for this issue (which we will keep in the future, to deal with 3rd party libraries that weren't regenerated using the new generator) but the real fix is to make the generator instead output code equivalent to:

  static void n_SetChildrenDrawingOrderEnabled_Z (IntPtr jnienv, IntPtr native__this, byte enabled)
  {
    var __this = global::Java.Lang.Object.GetObject<Android.Views.ViewGroup> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
    __this.ChildrenDrawingOrderEnabled = enabled != 0;
  }

  protected virtual unsafe bool ChildrenDrawingOrderEnabled {
    [Register ("setChildrenDrawingOrderEnabled", "(Z)V", "GetSetChildrenDrawingOrderEnabled_ZHandler")]
    set {
      // ...
    }
  }

or, for a method which returns a bool:

static byte n_IsMarginRelative (IntPtr jnienv, IntPtr native__this)
{
  var __this = global::Java.Lang.Object.GetObject<Android.Views.ViewGroup.MarginLayoutParams> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
  return __this.IsMarginRelative ? 1 : 0;
}

public virtual unsafe bool IsMarginRelative {
  [Register ("isMarginRelative", "()Z", "GetIsMarginRelativeHandler")]
  get {
    // ...
  }
}

The reason to choose byte is that JNI's jboolean type is defined as an unsigned 8-bit value and the reason to explicitly return 1 or 0 instead of
just casting the managed bool value is that the boolean type in dotnet always has value of 0 for false, but can have -1, 1 or != 0 for true,
depending on the VM or context and so it's safer to "downcast" that set to the 0/1 values common in other languages (including Java, C and C++ about
which we care)

In Mono.Android generated MCW code we currently have 4824 native callback methods either returning bool or with at least one bool parameter.

The generator should take into account other System namespace non-blittable types, not just bool (within reason - only those that can potentially happen in bindings). The list can be found in this table,
for reference copied below:

Non-blittable type Description
System.Array Converts to a C-style array or a SAFEARRAY.
System.Boolean Converts to a 1, 2, or 4-byte value with true as 1 or -1.
System.Char Converts to a Unicode or ANSI character.
System.Class Converts to a class interface.
System.Object Converts to a variant or an interface.
System.Mdarray Converts to a C-style array or a SAFEARRAY.
System.String Converts to a string terminating in a null reference or to a BSTR.
System.Valuetype Converts to a structure with a fixed memory layout.
System.Szarray Converts to a C-style array or a SAFEARRAY.
@grendello grendello added the generator Issues binding a Java library (generator, class-parse, etc.) label Aug 16, 2022
@jpobst jpobst added this to the 8.0.0 milestone Aug 22, 2022
grendello added a commit to xamarin/xamarin-android that referenced this issue Sep 7, 2022
Context: 903ba37
Context: e1af958
Context: xamarin/java.interop#1027

Commit 903ba37 mentioned a TODO:
    
> Update/rewrite infrastructure to focus on implementing the runtime
> side of marshal methods, making it possible to actually run
> applications which use marshal methods.

Implement the necessary runtime elements to enable running of
applications with marshal methods.  It is now possible, if
LLVM marshal methods are enabled/`ENABLE_MARSHAL_METHODS` is defined,
to run both plain .NET SDK for Android and MAUI apps.

Update `src/Microsoft.Android.Sdk.ILLink/PreserveLists/System.Runtime.InteropServices.xml`
so that `System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute`
is always preserved, as it is required by LLVM Marshal Methods.

The `[UnmanagedCallersOnly]` attribute used by marshal methods
requires that the invoked method have [blittable types][0] for the
method return type and all parameter types.  Unfortunately, `bool`
is *not* a blittable type.  Implement generation of wrapper methods
which replace `bool` with `byte` and convert the values appropriately
before calling the actual target method.  In a "hello world" MAUI
test app there are 133 such methods (out of around 180 total).

Implement code that enables us to show error messages with the proper
assembly, class and method names on failure to look up or obtain
pointer to native callback methods.

TODO:

  * Process *all* assemblies, including `Mono.Android.dll`, for
    Java Callable Wrapper generation.  This is necessary so that we
    can find and emit LLVM marshal methods for types defined within
    `Mono.Android.dll`.

  * Remove the `ENABLE_MARSHAL_METHODS` define, and enable
    LLVM marshal methods for everyone.

  * Update `<GenerateJavaStubs/>` to rewrite all assemblies for all
    Supported ABIs.  Currently, we don't support `Java.Lang.Object` &
    `Java.Lang.Throwable` subclasses being located in per-ABI
    assemblies.

  * How do `Java_…` native functions interact with
    `JNIEnv::RegisterNatives()`?
    #7285 (comment)

  * *Can* JNI `native` methods contain "non-printable" characters
    such as `\n`, or "non-representable in ELF symbols" characters
    such as `-` (e.g. Kotlin mangled methods)?
    #7285 (comment)

  * Cleanup, cleanup, cleanup

[0]: https://docs.microsoft.com/en-us/dotnet/framework/interop/blittable-and-non-blittable-types
@jpobst jpobst added the enhancement Proposed change to current functionality label Sep 12, 2022
@jpobst jpobst modified the milestones: 8.0.0, 9.0.0 Planning Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Proposed change to current functionality generator Issues binding a Java library (generator, class-parse, etc.)
Projects
None yet
Development

No branches or pull requests

2 participants