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

Calling from C++ into Java with local_ref may crash #34

Open
shelefg opened this issue Jul 7, 2020 · 2 comments
Open

Calling from C++ into Java with local_ref may crash #34

shelefg opened this issue Jul 7, 2020 · 2 comments

Comments

@shelefg
Copy link

shelefg commented Jul 7, 2020

Issue description

If a C++ call is made to a Java method whose reference is obtained by getMethod with local_ref arguments types, and that call throws an exception, it causes the process to crash. fbjni does not prevent specifying Java methods descriptors arguments in terms of local_ref.

This will only happen in debug builds because the crash is caused by a call to verifyReference in local_ref dtor, that is made inside an assert statement. verifyReference calls GetObjectRefType which is not allowed when there's a pending Java exception.

For release builds there is no issue because the actual JNI call to destroy a reference (DeleteLocalRef) is safe to call when there is a pending Java exception.

Code example

class JFoo : public facebook::jni::JavaClass<JFoo> {
  public:
  static constexpr auto kJavaDescriptor = "Lcom/example/Foo;";

  static void bar(facebook::jni::local_ref<JObject> jObj) {
    static auto method = javaClassStatic()->
                    getMethod<void(facebook::jni::local_ref<JObject>)>(
                    "bar");

    method(javaClassStatic(), jObj);
  }
};
package com.example
object Foo {
  @JvmStatic
  void bar(obj: Any) {
    throw Exception()
  }
}

Now a call such as this will crash:

JFoo::bar(facebook::jni::make_jstring(""));
@dreiss
Copy link
Contributor

dreiss commented Aug 18, 2020

Thanks for the very clear bug report. If you change the getMethod call to getMethod<void(JObject)>, does that fix the issue?

@shelefg
Copy link
Author

shelefg commented Aug 18, 2020

The following constructs seem to work:

getMethod<void(facebook::jni::alias_ref<JObject>)>

and

getMethod<void(JObject::javaobject)> 

The second requires calling get() on the jObj reference:

method(javaClassStatic(), jObj.get());

I think I saw both of these constructs used in React Native (which I used as an fbjni reference).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants