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

fix: Correctly resolve classes with FindClass(..) #34533

Closed
wants to merge 1 commit into from

Conversation

evancharlton
Copy link
Contributor

Summary

JNIEnv's FindClass(..) function takes the classes in the standard
foo/bar/Baz class specification (unless they're special, like arrays).
Specifying them with Lfoo/bar/Baz; results in a
ClassNotFoundException being raised -- which is especially unhelpful
when intending to re-throw an exception.

The docs for JNIEnv#FindClass(..) can be found here.

Changelog

[Android] [Fixed] - Correctly resolve classes with FindClass(..)

Test Plan

No exact test plan. However, if an exception is thrown during the
rendering process, a fatal ClassNotFoundException is raised, rather
than having the error be passed up to the ReactNativeManager.

Fixes: facebook/yoga#1120

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Microsoft Partner: Microsoft Partner labels Aug 30, 2022
`JNIEnv`'s `FindClass(..)` function takes the classes in the standard
`foo/bar/Baz` class specification (unless they're special, like arrays).
Specifying them with `Lfoo/bar/Baz;` results in a
`ClassNotFoundException` being raised -- which is especially unhelpful
when intending to re-throw an exception.

Fixes: facebook/yoga#1120
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,618,120 +7
android hermes armeabi-v7a 7,032,379 +15
android hermes x86 7,918,152 -14
android hermes x86_64 7,891,785 +3
android jsc arm64-v8a 9,495,473 +9
android jsc armeabi-v7a 8,273,105 +17
android jsc x86 9,433,231 -18
android jsc x86_64 10,026,237 +3

Base commit: 3afef3c
Branch: main

@evancharlton evancharlton marked this pull request as ready for review August 30, 2022 11:10
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 9e169da
Branch: main

@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Aug 30, 2022
@facebook-github-bot
Copy link
Contributor

@jacdebug has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot pushed a commit to facebook/litho that referenced this pull request Aug 31, 2022
Summary:
`JNIEnv`'s `FindClass(..)` function takes the classes in the standard
`foo/bar/Baz` class specification (unless they're special, like arrays).
Specifying them with `Lfoo/bar/Baz;` results in a
`ClassNotFoundException` being raised -- which is especially unhelpful
when intending to re-throw an exception.

The docs for `JNIEnv#FindClass(..)` can be found [here][jnienv].

[jnienv]:
  https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/functions.html#:~:text=The%20name%20argument,java/lang/String%22

## Changelog

[Android] [Fixed] - Correctly resolve classes with FindClass(..)

X-link: facebook/react-native#34533

Reviewed By: amir-shalem

Differential Revision: D39133326

Pulled By: jacdebug

fbshipit-source-id: 86283b7d21aed49ed0e9027b2aef85f0108cdf9a
facebook-github-bot pushed a commit to facebook/yoga that referenced this pull request Aug 31, 2022
Summary:
`JNIEnv`'s `FindClass(..)` function takes the classes in the standard
`foo/bar/Baz` class specification (unless they're special, like arrays).
Specifying them with `Lfoo/bar/Baz;` results in a
`ClassNotFoundException` being raised -- which is especially unhelpful
when intending to re-throw an exception.

The docs for `JNIEnv#FindClass(..)` can be found [here][jnienv].

[jnienv]:
  https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/functions.html#:~:text=The%20name%20argument,java/lang/String%22

## Changelog

[Android] [Fixed] - Correctly resolve classes with FindClass(..)

X-link: facebook/react-native#34533

Reviewed By: amir-shalem

Differential Revision: D39133326

Pulled By: jacdebug

fbshipit-source-id: 86283b7d21aed49ed0e9027b2aef85f0108cdf9a
@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @evancharlton in 361b310.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Aug 31, 2022
@evancharlton evancharlton deleted the bug/findclass branch August 31, 2022 06:14
dmytrorykun pushed a commit that referenced this pull request Sep 14, 2022
Summary:
`JNIEnv`'s `FindClass(..)` function takes the classes in the standard
`foo/bar/Baz` class specification (unless they're special, like arrays).
Specifying them with `Lfoo/bar/Baz;` results in a
`ClassNotFoundException` being raised -- which is especially unhelpful
when intending to re-throw an exception.

The docs for `JNIEnv#FindClass(..)` can be found [here][jnienv].

[jnienv]:
  https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/functions.html#:~:text=The%20name%20argument,java/lang/String%22

## Changelog

[Android] [Fixed] - Correctly resolve classes with FindClass(..)

Pull Request resolved: #34533

Test Plan:
No exact test plan. However, if an exception is thrown during the
rendering process, a fatal `ClassNotFoundException` is raised, rather
than having the error be passed up to the `ReactNativeManager`.

Fixes: facebook/yoga#1120

Reviewed By: amir-shalem

Differential Revision: D39133326

Pulled By: jacdebug

fbshipit-source-id: 86283b7d21aed49ed0e9027b2aef85f0108cdf9a
Titozzz pushed a commit that referenced this pull request Sep 26, 2022
Summary:
`JNIEnv`'s `FindClass(..)` function takes the classes in the standard
`foo/bar/Baz` class specification (unless they're special, like arrays).
Specifying them with `Lfoo/bar/Baz;` results in a
`ClassNotFoundException` being raised -- which is especially unhelpful
when intending to re-throw an exception.

The docs for `JNIEnv#FindClass(..)` can be found [here][jnienv].

[jnienv]:
  https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/functions.html#:~:text=The%20name%20argument,java/lang/String%22

## Changelog

[Android] [Fixed] - Correctly resolve classes with FindClass(..)

Pull Request resolved: #34533

Test Plan:
No exact test plan. However, if an exception is thrown during the
rendering process, a fatal `ClassNotFoundException` is raised, rather
than having the error be passed up to the `ReactNativeManager`.

Fixes: facebook/yoga#1120

Reviewed By: amir-shalem

Differential Revision: D39133326

Pulled By: jacdebug

fbshipit-source-id: 86283b7d21aed49ed0e9027b2aef85f0108cdf9a
Titozzz pushed a commit that referenced this pull request Oct 10, 2022
Summary:
`JNIEnv`'s `FindClass(..)` function takes the classes in the standard
`foo/bar/Baz` class specification (unless they're special, like arrays).
Specifying them with `Lfoo/bar/Baz;` results in a
`ClassNotFoundException` being raised -- which is especially unhelpful
when intending to re-throw an exception.

The docs for `JNIEnv#FindClass(..)` can be found [here][jnienv].

[jnienv]:
  https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/functions.html#:~:text=The%20name%20argument,java/lang/String%22

## Changelog

[Android] [Fixed] - Correctly resolve classes with FindClass(..)

Pull Request resolved: #34533

Test Plan:
No exact test plan. However, if an exception is thrown during the
rendering process, a fatal `ClassNotFoundException` is raised, rather
than having the error be passed up to the `ReactNativeManager`.

Fixes: facebook/yoga#1120

Reviewed By: amir-shalem

Differential Revision: D39133326

Pulled By: jacdebug

fbshipit-source-id: 86283b7d21aed49ed0e9027b2aef85f0108cdf9a
diegolmello pushed a commit to RocketChat/react-native that referenced this pull request Feb 2, 2023
Summary:
`JNIEnv`'s `FindClass(..)` function takes the classes in the standard
`foo/bar/Baz` class specification (unless they're special, like arrays).
Specifying them with `Lfoo/bar/Baz;` results in a
`ClassNotFoundException` being raised -- which is especially unhelpful
when intending to re-throw an exception.

The docs for `JNIEnv#FindClass(..)` can be found [here][jnienv].

[jnienv]:
  https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/functions.html#:~:text=The%20name%20argument,java/lang/String%22

## Changelog

[Android] [Fixed] - Correctly resolve classes with FindClass(..)

Pull Request resolved: facebook#34533

Test Plan:
No exact test plan. However, if an exception is thrown during the
rendering process, a fatal `ClassNotFoundException` is raised, rather
than having the error be passed up to the `ReactNativeManager`.

Fixes: facebook/yoga#1120

Reviewed By: amir-shalem

Differential Revision: D39133326

Pulled By: jacdebug

fbshipit-source-id: 86283b7d21aed49ed0e9027b2aef85f0108cdf9a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Microsoft Partner: Microsoft Partner Platform: Android Android applications. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enabling logging on Android crashes the app
4 participants