Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add OnEnterMarshalMethod code.
  • Loading branch information
jpobst committed Dec 4, 2024
commit 1d2fab9a651f4e652d919ef330d4ed98fed41b27
5 changes: 5 additions & 0 deletions src/Java.Interop/Java.Interop/JniRuntime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,11 @@ public virtual bool ExceptionShouldTransitionToJni (Exception e)

partial class JniRuntime {

public virtual void OnEnterMarshalMethod ()
{
ValueManager.WaitForGCBridgeProcessing ();
}

public virtual void OnUserUnhandledException (ref JniTransition transition, Exception e)
{
transition.SetPendingException (e);
Expand Down
1 change: 1 addition & 0 deletions src/Java.Interop/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
#nullable enable
virtual Java.Interop.JniRuntime.OnEnterMarshalMethod() -> void
virtual Java.Interop.JniRuntime.OnUserUnhandledException(ref Java.Interop.JniTransition transition, System.Exception! e) -> void
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,14 @@ internal partial class AnimatorListenerInvoker : global::Java.Lang.Object, Anima
static bool n_OnAnimationEnd_I (IntPtr jnienv, IntPtr native__this, int param1)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should have called this out more than just in the draft commit message, but the problem with this is that JniEnvironment.Runtime can throw:

get => runtime ?? throw new NotSupportedException ();

It shouldn't throw, ideally, but it could, if e.g. the runtime hasn't been initialized yet, or if the runtime has been shutdown.

If it throws an exception outside of the try/catch, we can unwind past the Java frames, and corrupt the JVM.

We could ignore this possibility, and keep this as-is, or we can go with what the draft message suggested:

var __envp = new global::Java.Interop.JniTransition (jnienv);
global::Java.Interop.JniRuntime? __r = null;
try {
  __r = global::Java.Interop.JniEnvironment.Runtime;
}

and alternate approach would be to introduce more public methods:

namespace Java.Interop;

partial class JniEnvironment {
    public static bool BeginMarshalMethod (IntPtr jnienv, out JniTransition envp, [NotNullWhen(true)] out JniRuntime? runtime);
    public static void EndMarshalMethod (ref JniTransition envp);
}

allowing:

if (!JniEnvironment.BeginMarshalMethod (jnienv, out var __envp, out var __r))
    return /* default */;
try {
    __r.OnEnterMarshalMethod ();
    // …
} catch (Exception e) {
    __r.OnUserUnandledException (ref __e, e);
} finally {
    JniEnvironment.EndMarshalMethod (ref __envp);
}

A question that comes to mind is IL size: which is smaller, between:

  • "just call JniEnvironment.Runtime"
  • Call JniEnvironment.Runtime within the try block
  • JniEnvironment.BeginMarshalMethod()

Enter my sample app (see below).

  • A(): "just call JniEnvironment.Runtime"
    Code size 44
  • B(): Call JniEnvironment.Runtime within the try block
    Code size: 49
  • D(): JniEnvironment.BeginMarshalMethod()
    Code size: 43

which makes it look like JniEnvironment.BeginMarshalMethod() + JniEnvironment.EndMarshalMethod() has the smallest impact on code size, while still permitting correctness.

// See https://aka.ms/new-console-template for more information
Console.WriteLine("Hello, World!");

void A ()
{
    var __e = new JniTransition (0);
    var r = JniEnvironment.Runtime;
    try {
        r.OnEnterMarshalMethod();
    } catch (Exception e) {
        r.OnUserUnandledException (ref __e, e);
    } finally {
        __e.Dispose();
    }
}

void B ()
{
    var __e = new JniTransition (0);
    JniRuntime? r = null;
    try {
        r = JniEnvironment.Runtime;
        r.OnEnterMarshalMethod();
    } catch (Exception e) {
        r?.OnUserUnandledException (ref __e, e);
    } finally {
        __e.Dispose();
    }
}

void C ()
{
    if (!JniEnvironment.BeginMarshalMethod (0, out var __e, out var r))
        return;
    try {
        r.OnEnterMarshalMethod();
    } catch (Exception e) {
        r.OnUserUnandledException (ref __e, e);
    } finally {
        __e.Dispose();
    }
}

void D ()
{
    if (!JniEnvironment.BeginMarshalMethod (0, out var __e, out var r))
        return;
    try {
        r.OnEnterMarshalMethod();
    } catch (Exception e) {
        r.OnUserUnandledException (ref __e, e);
    } finally {
        JniEnvironment.EndMarshalMethod (ref __e);
    }
}

struct JniTransition {
    public JniTransition (IntPtr env) {}
    public void Dispose() {}
}

class JniEnvironment {
    public static JniRuntime Runtime => throw null;
    public static bool BeginMarshalMethod (int env, out JniTransition __e, out JniRuntime r) => throw null;
    public static void EndMarshalMethod (ref JniTransition __e) => throw null;
}

class JniRuntime {
    public class JniValueManager {

    }

    public virtual void OnUserUnandledException (ref JniTransition env, Exception e)
    {
    }

    public virtual void OnEnterMarshalMethod ()
    {
    }

    public JniValueManager ValueManager => throw null!;
}


try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<java.code.AnimatorListener> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
return __this.OnAnimationEnd (param1);
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
return default;
} finally {
__envp.Dispose ();
Expand Down Expand Up @@ -111,12 +113,14 @@ internal partial class AnimatorListenerInvoker : global::Java.Lang.Object, Anima
static bool n_OnAnimationEnd_II (IntPtr jnienv, IntPtr native__this, int param1, int param2)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;

try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<java.code.AnimatorListener> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
return __this.OnAnimationEnd (param1, param2);
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
return default;
} finally {
__envp.Dispose ();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,14 @@ internal partial class IMyInterface2Invoker : global::Java.Lang.Object, IMyInter
static void n_DoSomething (IntPtr jnienv, IntPtr native__this)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;

try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<java.code.IMyInterface2> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
__this.DoSomething ();
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
} finally {
__envp.Dispose ();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,14 @@ public partial class MyClass {
static int n_get_Count (IntPtr jnienv, IntPtr native__this)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;

try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<java.code.MyClass> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
return __this.Count;
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
return default;
} finally {
__envp.Dispose ();
Expand All @@ -84,12 +86,14 @@ public partial class MyClass {
static void n_set_Count_I (IntPtr jnienv, IntPtr native__this, int value)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;

try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<java.code.MyClass> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
__this.Count = value;
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
} finally {
__envp.Dispose ();
}
Expand Down Expand Up @@ -131,12 +135,14 @@ public partial class MyClass {
static IntPtr n_get_Key (IntPtr jnienv, IntPtr native__this)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;

try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<java.code.MyClass> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
return JNIEnv.NewString (__this.Key);
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
return default;
} finally {
__envp.Dispose ();
Expand All @@ -155,13 +161,15 @@ public partial class MyClass {
static void n_set_Key_Ljava_lang_String_ (IntPtr jnienv, IntPtr native__this, IntPtr native_value)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;

try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<java.code.MyClass> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
var value = JNIEnv.GetString (native_value, JniHandleOwnership.DoNotTransfer);
__this.Key = value;
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
} finally {
__envp.Dispose ();
}
Expand Down Expand Up @@ -229,12 +237,14 @@ public partial class MyClass {
static int n_get_AbstractCount (IntPtr jnienv, IntPtr native__this)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;

try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<java.code.MyClass> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
return __this.AbstractCount;
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
return default;
} finally {
__envp.Dispose ();
Expand All @@ -253,12 +263,14 @@ public partial class MyClass {
static void n_set_AbstractCount_I (IntPtr jnienv, IntPtr native__this, int value)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;

try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<java.code.MyClass> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
__this.AbstractCount = value;
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
} finally {
__envp.Dispose ();
}
Expand Down Expand Up @@ -286,14 +298,16 @@ public partial class MyClass {
static int n_GetCountForKey_Ljava_lang_String_ (IntPtr jnienv, IntPtr native__this, IntPtr native_key)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;

try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<java.code.MyClass> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
var key = JNIEnv.GetString (native_key, JniHandleOwnership.DoNotTransfer);
int __ret = __this.GetCountForKey (key);
return __ret;
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
return default;
} finally {
__envp.Dispose ();
Expand Down Expand Up @@ -328,12 +342,14 @@ public partial class MyClass {
static IntPtr n_Key (IntPtr jnienv, IntPtr native__this)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;

try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<java.code.MyClass> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
return JNIEnv.NewString (__this.Key ());
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
return default;
} finally {
__envp.Dispose ();
Expand Down Expand Up @@ -375,12 +391,14 @@ public partial class MyClass {
static void n_AbstractMethod (IntPtr jnienv, IntPtr native__this)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;

try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<java.code.MyClass> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
__this.AbstractMethod ();
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
} finally {
__envp.Dispose ();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,12 +141,14 @@ internal partial class IMyInterfaceInvoker : global::Java.Lang.Object, IMyInterf
static int n_get_Count (IntPtr jnienv, IntPtr native__this)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;

try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<java.code.IMyInterface> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
return __this.Count;
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
return default;
} finally {
__envp.Dispose ();
Expand All @@ -165,12 +167,14 @@ internal partial class IMyInterfaceInvoker : global::Java.Lang.Object, IMyInterf
static void n_set_Count_I (IntPtr jnienv, IntPtr native__this, int value)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;

try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<java.code.IMyInterface> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
__this.Count = value;
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
} finally {
__envp.Dispose ();
}
Expand Down Expand Up @@ -205,12 +209,14 @@ internal partial class IMyInterfaceInvoker : global::Java.Lang.Object, IMyInterf
static IntPtr n_get_Key (IntPtr jnienv, IntPtr native__this)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;

try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<java.code.IMyInterface> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
return JNIEnv.NewString (__this.Key);
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
return default;
} finally {
__envp.Dispose ();
Expand All @@ -229,13 +235,15 @@ internal partial class IMyInterfaceInvoker : global::Java.Lang.Object, IMyInterf
static void n_set_Key_Ljava_lang_String_ (IntPtr jnienv, IntPtr native__this, IntPtr native_value)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;

try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<java.code.IMyInterface> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
var value = JNIEnv.GetString (native_value, JniHandleOwnership.DoNotTransfer);
__this.Key = value;
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
} finally {
__envp.Dispose ();
}
Expand Down Expand Up @@ -272,12 +280,14 @@ internal partial class IMyInterfaceInvoker : global::Java.Lang.Object, IMyInterf
static int n_get_AbstractCount (IntPtr jnienv, IntPtr native__this)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;

try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<java.code.IMyInterface> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
return __this.AbstractCount;
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
return default;
} finally {
__envp.Dispose ();
Expand All @@ -296,12 +306,14 @@ internal partial class IMyInterfaceInvoker : global::Java.Lang.Object, IMyInterf
static void n_set_AbstractCount_I (IntPtr jnienv, IntPtr native__this, int value)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;

try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<java.code.IMyInterface> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
__this.AbstractCount = value;
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
} finally {
__envp.Dispose ();
}
Expand Down Expand Up @@ -336,14 +348,16 @@ internal partial class IMyInterfaceInvoker : global::Java.Lang.Object, IMyInterf
static int n_GetCountForKey_Ljava_lang_String_ (IntPtr jnienv, IntPtr native__this, IntPtr native_key)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;

try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<java.code.IMyInterface> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
var key = JNIEnv.GetString (native_key, JniHandleOwnership.DoNotTransfer);
int __ret = __this.GetCountForKey (key);
return __ret;
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
return default;
} finally {
__envp.Dispose ();
Expand Down Expand Up @@ -375,12 +389,14 @@ internal partial class IMyInterfaceInvoker : global::Java.Lang.Object, IMyInterf
static IntPtr n_Key (IntPtr jnienv, IntPtr native__this)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;

try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<java.code.IMyInterface> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
return JNIEnv.NewString (__this.Key ());
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
return default;
} finally {
__envp.Dispose ();
Expand All @@ -407,12 +423,14 @@ internal partial class IMyInterfaceInvoker : global::Java.Lang.Object, IMyInterf
static void n_AbstractMethod (IntPtr jnienv, IntPtr native__this)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;

try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<java.code.IMyInterface> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
__this.AbstractMethod ();
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
} finally {
__envp.Dispose ();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,18 @@ public partial class MyClass : Java.Lang.Object {
static IntPtr n_Echo_arrayLjava_lang_CharSequence_ (IntPtr jnienv, IntPtr native__this, IntPtr native_messages)
{
var __envp = new global::Java.Interop.JniTransition (jnienv);
var __r = global::Java.Interop.JniEnvironment.Runtime;

try {
__r.OnEnterMarshalMethod ();
var __this = global::Java.Lang.Object.GetObject<Com.Example.MyClass> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
var messages = (Java.Lang.ICharSequence[]?) JNIEnv.GetArray (native_messages, JniHandleOwnership.DoNotTransfer, typeof (Java.Lang.ICharSequence));
IntPtr __ret = JNIEnv.NewArray (__this.EchoFormatted (messages));
if (messages != null)
JNIEnv.CopyArray (messages, native_messages);
return __ret;
} catch (global::System.Exception __e) {
global::Java.Interop.JniEnvironment.Runtime.OnUserUnhandledException (ref __envp, __e);
__r.OnUserUnhandledException (ref __envp, __e);
return default;
} finally {
__envp.Dispose ();
Expand Down
Loading