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 a bug where Netty fails to load a shaded native library #12358

Merged
merged 3 commits into from May 5, 2022

Conversation

trustin
Copy link
Member

@trustin trustin commented May 4, 2022

Related: netty/netty-jni-util#13

Motivation:

Netty can't load a shaded native library because netty-jni-util has a
bug that appends an extra / when attmpting JNI FindClass. For
example, parsePackagePrefix() returns shaded// instead of shaded/,
leading to a bad attempt to load shaded//io/netty/....

Netty also doesn't handle the case where a shaded package name contains
an underscore (_), such as my_shaded_deps.io.netty.*, because it
simply replaces . with _ and vice versa. JNI specification defines
a mangling rule to avoid this issue:

Modifications:

  • Update netty-jni-utils to 0.0.6.Final which contains the fix for
    parsePackagePrefix().
  • Replace _ into _1 so that parsePackagePrefix() in
    netty-jni-utils.c can get the correct package name later.
  • Update the docker-compose.yaml so that the integration tests in
    testsuite-shading are always run as a part of CI, so we don't have
    a regression in the future.

Result:

Note:

  • This PR will not pass until we:
    • release netty-jni-util 0.0.6.Final
    • update netty-tcnative and this PR to depend on
      netty-jni-util-0.0.6.Final; and
    • release a new version of netty-tcnative.

Related: netty/netty-jni-util#13

Motivation:

Netty can't load a shaded native library because `netty-jni-util` has a
bug that appends an extra `/` when attmpting JNI `FindClass`. For
example, `parsePackagePrefix()` returns `shaded//` instead of `shaded/`,
leading to a bad attempt to load `shaded//io/netty/...`.

Netty also doesn't handle the case where a shaded package name contains
an underscore (`_`), such as `my_shaded_deps.io.netty.*`, because it
simply replaces `.` with `_` and vice versa. JNI specification defines
a mangling rule to avoid this issue:

- https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/design.html#resolving_native_method_names

Modifications:

- Replace `_` into `_1` so that `parsePackagePrefix()` in
  `netty-jni-utils.c` can get the correct package name later.
- Update the `docker-compose.yaml` so that the integration tests in
  `testsuite-shading` are always run as a part of CI, so we don't have
  a regression in the future.

Result:

- A user can use a functionality that requires a native library even if
  they shaded Netty *and* the shaded package name contains an underscore
  (`_`).

Note:

- This PR will not pass until we:
  - release `netty-jni-util 0.0.6.Final`
  - update `netty-tcnative` and this PR to depend on
    `netty-jni-util-0.0.6.Final`; and
  - release a new version of `netty-tcnative`.
@trustin trustin added defect regression ci Issues related to CI setup (GH Actions) labels May 4, 2022
@trustin trustin added this to the 4.1.77.Final milestone May 4, 2022
@trustin trustin requested a review from normanmaurer May 4, 2022 10:01
normanmaurer pushed a commit to netty/netty-tcnative that referenced this pull request May 4, 2022
Motivation:

We did not correctly escape chars when shading is used.

Modifications:

- Update jni-utils to fix bug

Result:

Fixes escaping

Related:
- #420
- netty/netty-jni-util#13
- netty/netty#12358
normanmaurer added a commit to netty/netty-incubator-codec-quic that referenced this pull request May 5, 2022
Motivation:

We did not correctly escape chars when shading is used.

Modifications:

- Update jni-utils to fix bug

Result:

Fixes escaping

Related:
- netty/netty-jni-util#13
- netty/netty#12358
normanmaurer added a commit to netty/netty-incubator-codec-quic that referenced this pull request May 5, 2022
Motivation:

We did not correctly escape chars when shading is used.

Modifications:

- Update jni-utils to fix bug

Result:

Fixes escaping

Related:
- netty/netty-jni-util#13
- netty/netty#12358
@normanmaurer normanmaurer merged commit eb00590 into 4.1 May 5, 2022
@normanmaurer normanmaurer deleted the fix_parse_package_prefix branch May 5, 2022 09:53
normanmaurer pushed a commit that referenced this pull request May 5, 2022
Related: netty/netty-jni-util#13

Motivation:

Netty can't load a shaded native library because `netty-jni-util` has a
bug that appends an extra `/` when attmpting JNI `FindClass`. For
example, `parsePackagePrefix()` returns `shaded//` instead of `shaded/`,
leading to a bad attempt to load `shaded//io/netty/...`.

Netty also doesn't handle the case where a shaded package name contains
an underscore (`_`), such as `my_shaded_deps.io.netty.*`, because it
simply replaces `.` with `_` and vice versa. JNI specification defines
a mangling rule to avoid this issue:

- https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/design.html#resolving_native_method_names

Modifications:

- Replace `_` into `_1` so that `parsePackagePrefix()` in
  `netty-jni-utils.c` can get the correct package name later.
- Update the `docker-compose.yaml` so that the integration tests in
  `testsuite-shading` are always run as a part of CI, so we don't have
  a regression in the future.

Result:

- A user can use a functionality that requires a native library even if
  they shaded Netty *and* the shaded package name contains an underscore
  (`_`).
normanmaurer added a commit that referenced this pull request May 6, 2022
…12362)

Related: netty/netty-jni-util#13

Motivation:

Netty can't load a shaded native library because `netty-jni-util` has a
bug that appends an extra `/` when attmpting JNI `FindClass`. For
example, `parsePackagePrefix()` returns `shaded//` instead of `shaded/`,
leading to a bad attempt to load `shaded//io/netty/...`.

Netty also doesn't handle the case where a shaded package name contains
an underscore (`_`), such as `my_shaded_deps.io.netty.*`, because it
simply replaces `.` with `_` and vice versa. JNI specification defines
a mangling rule to avoid this issue:

- https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/design.html#resolving_native_method_names

Modifications:

- Replace `_` into `_1` so that `parsePackagePrefix()` in
  `netty-jni-utils.c` can get the correct package name later.
- Update the `docker-compose.yaml` so that the integration tests in
  `testsuite-shading` are always run as a part of CI, so we don't have
  a regression in the future.

Result:

- A user can use a functionality that requires a native library even if
  they shaded Netty *and* the shaded package name contains an underscore
  (`_`).

Co-authored-by: Trustin Lee <t@motd.kr>
@chrisvest
Copy link
Contributor

@normanmaurer The main branch build has been failing since this got merged.

@normanmaurer
Copy link
Member

Will check

raidyue pushed a commit to raidyue/netty that referenced this pull request Jul 8, 2022
)


Related: netty/netty-jni-util#13

Motivation:

Netty can't load a shaded native library because `netty-jni-util` has a
bug that appends an extra `/` when attmpting JNI `FindClass`. For
example, `parsePackagePrefix()` returns `shaded//` instead of `shaded/`,
leading to a bad attempt to load `shaded//io/netty/...`.

Netty also doesn't handle the case where a shaded package name contains
an underscore (`_`), such as `my_shaded_deps.io.netty.*`, because it
simply replaces `.` with `_` and vice versa. JNI specification defines
a mangling rule to avoid this issue:

- https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/design.html#resolving_native_method_names

Modifications:

- Replace `_` into `_1` so that `parsePackagePrefix()` in
  `netty-jni-utils.c` can get the correct package name later.
- Update the `docker-compose.yaml` so that the integration tests in
  `testsuite-shading` are always run as a part of CI, so we don't have
  a regression in the future.

Result:

- A user can use a functionality that requires a native library even if
  they shaded Netty *and* the shaded package name contains an underscore
  (`_`).
franz1981 pushed a commit to franz1981/netty that referenced this pull request Aug 22, 2022
)


Related: netty/netty-jni-util#13

Motivation:

Netty can't load a shaded native library because `netty-jni-util` has a
bug that appends an extra `/` when attmpting JNI `FindClass`. For
example, `parsePackagePrefix()` returns `shaded//` instead of `shaded/`,
leading to a bad attempt to load `shaded//io/netty/...`.

Netty also doesn't handle the case where a shaded package name contains
an underscore (`_`), such as `my_shaded_deps.io.netty.*`, because it
simply replaces `.` with `_` and vice versa. JNI specification defines
a mangling rule to avoid this issue:

- https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/design.html#resolving_native_method_names

Modifications:

- Replace `_` into `_1` so that `parsePackagePrefix()` in
  `netty-jni-utils.c` can get the correct package name later.
- Update the `docker-compose.yaml` so that the integration tests in
  `testsuite-shading` are always run as a part of CI, so we don't have
  a regression in the future.

Result:

- A user can use a functionality that requires a native library even if
  they shaded Netty *and* the shaded package name contains an underscore
  (`_`).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Issues related to CI setup (GH Actions) defect regression
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ShadingIT fails on macOS and Linux
3 participants