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

If using more than one shaded netty-tcnative library in a JVM, only the first loads successfully #7272

Closed
steveniemitz opened this Issue Oct 2, 2017 · 26 comments

Comments

Projects
None yet
9 participants
@steveniemitz

steveniemitz commented Oct 2, 2017

Now that netty can automatically detect the package prefix ( #6995 ), I attempted to shade netty into two different packages, lets call them A and B.

Package A loaded tcnative successfully (lets say it was shaded to A.repackaged.io.netty).

However, when package B attempted to load, I got an UnsatisfiedLinkError invoking aprMajorVersion() in io.netty.internal.tcnative.Library.

Digging in more, I do see the native library (from package B) being unpacked and loadLibrary being called on it (and not throwing an exception). However, lldb reports that it is not loaded (eg it doesn't show up in image list, only the library from package A does), and further, running the JVM with -verbose:jni shows the JNI native methods for the shaded version of tcnative in package A being registered (again) instead of B. ex:

[Registering JNI native method A.repackaged.io.netty.internal.tcnative.Library.initialize0]

I've tested this on OSX, but not Linux yet.

@johnou

This comment has been minimized.

Show comment
Hide comment
@johnou

johnou Oct 3, 2017

Contributor
Contributor

johnou commented Oct 3, 2017

@normanmaurer

This comment has been minimized.

Show comment
Hide comment
@normanmaurer
Member

normanmaurer commented Oct 4, 2017

@carl-mastrangelo

This comment has been minimized.

Show comment
Hide comment
@carl-mastrangelo

carl-mastrangelo Oct 4, 2017

Member

@steveniemitz to clarify, you mean the shaded version of io.netty.internal.tcnative.Library right? At leats on OpenJDK, most of the interesting Native library loading logic is in ClassLoader. Have you tried stepping through it in a debugger yet?

Member

carl-mastrangelo commented Oct 4, 2017

@steveniemitz to clarify, you mean the shaded version of io.netty.internal.tcnative.Library right? At leats on OpenJDK, most of the interesting Native library loading logic is in ClassLoader. Have you tried stepping through it in a debugger yet?

@steveniemitz

This comment has been minimized.

Show comment
Hide comment
@steveniemitz

steveniemitz Oct 4, 2017

Sorry, yeah my post was confusing there. I do mean the shaded version of Library.

Working from memory right now, I have stepped through. The call to (native) System.loadLibrary succeeds, and it does pass the expected correct shaded lib name. I haven't stepped though the native code though.

steveniemitz commented Oct 4, 2017

Sorry, yeah my post was confusing there. I do mean the shaded version of Library.

Working from memory right now, I have stepped through. The call to (native) System.loadLibrary succeeds, and it does pass the expected correct shaded lib name. I haven't stepped though the native code though.

@Xeli

This comment has been minimized.

Show comment
Hide comment
@Xeli

Xeli Oct 10, 2017

Sorry to add noise to this issue, but how did you manage to shade netty with tcnative at all? I run into the problem that netty cannot load the native library (for details: stackoverflow)

Xeli commented Oct 10, 2017

Sorry to add noise to this issue, but how did you manage to shade netty with tcnative at all? I run into the problem that netty cannot load the native library (for details: stackoverflow)

@ejona86

This comment has been minimized.

Show comment
Hide comment
@ejona86

ejona86 Oct 10, 2017

Contributor

@Xeli, you also need to rename the .so's, .jnilib, and .dll. See #6995 for some more information.

Contributor

ejona86 commented Oct 10, 2017

@Xeli, you also need to rename the .so's, .jnilib, and .dll. See #6995 for some more information.

@normanmaurer

This comment has been minimized.

Show comment
Hide comment
@normanmaurer

normanmaurer Nov 21, 2017

Member

@steveniemitz so can you please clarify what exactly the issue is ?

Member

normanmaurer commented Nov 21, 2017

@steveniemitz so can you please clarify what exactly the issue is ?

@steveniemitz

This comment has been minimized.

Show comment
Hide comment
@steveniemitz

steveniemitz Nov 21, 2017

Sorry, I'm not sure how I could be more descriptive, what is unclear here?

steveniemitz commented Nov 21, 2017

Sorry, I'm not sure how I could be more descriptive, what is unclear here?

@Scottmitch

This comment has been minimized.

Show comment
Hide comment
@Scottmitch

Scottmitch Nov 21, 2017

Member

However, when package B attempted to load, I got an UnsatisfiedLinkError invoking aprMajorVersion() in io.netty.internal.tcnative.Library.

As @ejona86 said #7272 (comment) you need to rename the native libraries. This is generally not done automatically by shading plugins for maven and gradle ... you need to configure an extra step.

Member

Scottmitch commented Nov 21, 2017

However, when package B attempted to load, I got an UnsatisfiedLinkError invoking aprMajorVersion() in io.netty.internal.tcnative.Library.

As @ejona86 said #7272 (comment) you need to rename the native libraries. This is generally not done automatically by shading plugins for maven and gradle ... you need to configure an extra step.

@steveniemitz

This comment has been minimized.

Show comment
Hide comment
@steveniemitz

steveniemitz Nov 21, 2017

Yes, I realize the native libraries need to be renamed. My original report specifically calls out that I see the correct library being unpacked and loadLibrary being called on it:

"Digging in more, I do see the native library (from package B) being unpacked and loadLibrary being called on it (and not throwing an exception)."

Both shaded jars work fine on their own. The problem manifests only after the first shaded library has loaded and registered its JNI methods.

steveniemitz commented Nov 21, 2017

Yes, I realize the native libraries need to be renamed. My original report specifically calls out that I see the correct library being unpacked and loadLibrary being called on it:

"Digging in more, I do see the native library (from package B) being unpacked and loadLibrary being called on it (and not throwing an exception)."

Both shaded jars work fine on their own. The problem manifests only after the first shaded library has loaded and registered its JNI methods.

@ejona86

This comment has been minimized.

Show comment
Hide comment
@ejona86

ejona86 Nov 21, 2017

Contributor

As far as investigation, I think we should just try the same setup as mentioned: two normally-incompatible tcnatives, with at least one shaded.

I think this is the sort of case I've been mentioning I was concerned about (offline). I've not had time to investigate, and it'll probably be time-consuming. I'm worried there won't be a solution, other than swapping to Conscrypt. Since Conscrypt (will have) a stable API, you don't shade, and so there's only one version in play.

Contributor

ejona86 commented Nov 21, 2017

As far as investigation, I think we should just try the same setup as mentioned: two normally-incompatible tcnatives, with at least one shaded.

I think this is the sort of case I've been mentioning I was concerned about (offline). I've not had time to investigate, and it'll probably be time-consuming. I'm worried there won't be a solution, other than swapping to Conscrypt. Since Conscrypt (will have) a stable API, you don't shade, and so there's only one version in play.

@steveniemitz

This comment has been minimized.

Show comment
Hide comment
@steveniemitz

steveniemitz Nov 21, 2017

fwiw, I was testing with (I think, this was over a month ago now)

  • google-cloud-trace library + tcnative 2.0.6 (shaded),
  • bigtable-client-core + tcnative 2.0.6 (shaded)

So the same version of the lib (also binary identical). My naive guess is the OSX loader is detecting that a native lib is being dlopened twice and simply ignoring the second one.

steveniemitz commented Nov 21, 2017

fwiw, I was testing with (I think, this was over a month ago now)

  • google-cloud-trace library + tcnative 2.0.6 (shaded),
  • bigtable-client-core + tcnative 2.0.6 (shaded)

So the same version of the lib (also binary identical). My naive guess is the OSX loader is detecting that a native lib is being dlopened twice and simply ignoring the second one.

@Scottmitch

This comment has been minimized.

Show comment
Hide comment
@Scottmitch

Scottmitch Nov 21, 2017

Member

Yes, I realize the native libraries need to be renamed.

@steveniemitz - my mistake ... I glanced too quickly at the issue description.

Member

Scottmitch commented Nov 21, 2017

Yes, I realize the native libraries need to be renamed.

@steveniemitz - my mistake ... I glanced too quickly at the issue description.

@Scottmitch

This comment has been minimized.

Show comment
Hide comment
@Scottmitch

Scottmitch Nov 21, 2017

Member

Here are some options which maybe viable:

https://stackoverflow.com/a/6540059

If the binaries are identical ... it maybe just sufficient to have the JNI_OnLoad method invoked, which will do the JNI registration under the shaded method names.

If the binaries are not identical we would want both libraries to be loaded to avoid conflicts. Have tried with different versions of tcnative @steveniemitz ?

Member

Scottmitch commented Nov 21, 2017

Here are some options which maybe viable:

https://stackoverflow.com/a/6540059

If the binaries are identical ... it maybe just sufficient to have the JNI_OnLoad method invoked, which will do the JNI registration under the shaded method names.

If the binaries are not identical we would want both libraries to be loaded to avoid conflicts. Have tried with different versions of tcnative @steveniemitz ?

@steveniemitz

This comment has been minimized.

Show comment
Hide comment
@steveniemitz

steveniemitz Nov 22, 2017

I have not tried two different versions. Based on my guess from above I expect it'd work though.

steveniemitz commented Nov 22, 2017

I have not tried two different versions. Based on my guess from above I expect it'd work though.

@acharis

This comment has been minimized.

Show comment
Hide comment
@acharis

acharis Jul 31, 2018

While others in my organization running OSX have run into this issue, I can confirm that on linux this works for me. Happy to give more information to clarify what I mean by "linux".

acharis commented Jul 31, 2018

While others in my organization running OSX have run into this issue, I can confirm that on linux this works for me. Happy to give more information to clarify what I mean by "linux".

@jhaber

This comment has been minimized.

Show comment
Hide comment
@jhaber

jhaber Jul 31, 2018

To expand on what Alex said, we ran into this issue in our apps that were using a mix of grpc-netty-shaded as well as another copy of shaded netty under a different package prefix (both copies are version 4.1.22.Final). This was the minimal reproducible example we came up with:

public static void main(String... args) throws Exception {
    System.out.println(io.grpc.netty.shaded.io.netty.handler.ssl.OpenSsl.isAvailable());
    System.out.println(other.netty.shaded.io.netty.handler.ssl.OpenSsl.isAvailable());
}

On OSX, whichever OpenSsl class is invoked first returns true, and the second always returns false. But it's not reproducible on the flavors of Linux that we've tried. We haven't tested on Windows.

jhaber commented Jul 31, 2018

To expand on what Alex said, we ran into this issue in our apps that were using a mix of grpc-netty-shaded as well as another copy of shaded netty under a different package prefix (both copies are version 4.1.22.Final). This was the minimal reproducible example we came up with:

public static void main(String... args) throws Exception {
    System.out.println(io.grpc.netty.shaded.io.netty.handler.ssl.OpenSsl.isAvailable());
    System.out.println(other.netty.shaded.io.netty.handler.ssl.OpenSsl.isAvailable());
}

On OSX, whichever OpenSsl class is invoked first returns true, and the second always returns false. But it's not reproducible on the flavors of Linux that we've tried. We haven't tested on Windows.

@normanmaurer

This comment has been minimized.

Show comment
Hide comment
@normanmaurer

normanmaurer Jul 31, 2018

Member

@acharis yes please... Can you also explain "this works for me" means ? Does it mean that everything "just works" as it is for you on linux ?

Member

normanmaurer commented Jul 31, 2018

@acharis yes please... Can you also explain "this works for me" means ? Does it mean that everything "just works" as it is for you on linux ?

@acharis

This comment has been minimized.

Show comment
Hide comment
@acharis

acharis Jul 31, 2018

Yes, sorry, I was far too terse.
Running jhaber's minimal example above prints

true
true

on my linux machine.

Additionally, members of my team have gotten that example to run on OSX by running OSX's install_name_tool to change the id of the dylib for our differently shaded netty.

acharis commented Jul 31, 2018

Yes, sorry, I was far too terse.
Running jhaber's minimal example above prints

true
true

on my linux machine.

Additionally, members of my team have gotten that example to run on OSX by running OSX's install_name_tool to change the id of the dylib for our differently shaded netty.

@normanmaurer

This comment has been minimized.

Show comment
Hide comment
@normanmaurer

normanmaurer Jul 31, 2018

Member

@acharis can you give some more details on what exactly they did with install_name_tool ?

Member

normanmaurer commented Jul 31, 2018

@acharis can you give some more details on what exactly they did with install_name_tool ?

@acharis

This comment has been minimized.

Show comment
Hide comment
@acharis

acharis Jul 31, 2018

install_name_tool -id my_id <path to dylib>

acharis commented Jul 31, 2018

install_name_tool -id my_id <path to dylib>

normanmaurer added a commit that referenced this issue Aug 20, 2018

Ensure multiple shaded version of the same netty artifact can be load…
…ed as long as the shaded prefix is different

Motivation:

We should support to load multiple shaded versions of the same netty artifact as netty is often used in multiple dependencies.

This is related to #7272.

Modifications:

- Use __attribute__((visibility("hidden"))) for things defined in header files but which should only be used internally
- Ensure fields are declared as static so these are not exported
- Adjust testsuite-shading to use install_name_tool on MacOS to change the id of the lib. Otherwise the wrong may be used.

Result:

Be able to use multiple shaded versions of the same netty artifact.
@normanmaurer

This comment has been minimized.

Show comment
Hide comment
@normanmaurer

normanmaurer Aug 20, 2018

Member

Please see #8207 ... I will do the same changes for tcnative as well.

Also I really think about writing a maven and gradle plugin to do all of the needed steps to shade netty / netty-tcnative so people will not need to mess with all of this. Thoughts ?

Member

normanmaurer commented Aug 20, 2018

Please see #8207 ... I will do the same changes for tcnative as well.

Also I really think about writing a maven and gradle plugin to do all of the needed steps to shade netty / netty-tcnative so people will not need to mess with all of this. Thoughts ?

normanmaurer added a commit that referenced this issue Aug 20, 2018

Ensure multiple shaded version of the same netty artifact can be load…
…ed as long as the shaded prefix is different

Motivation:

We should support to load multiple shaded versions of the same netty artifact as netty is often used in multiple dependencies.

This is related to #7272.

Modifications:

- Use -fvisibility=hidden when compiling and use __attribute__((visibility("default"))) for things we really want to have exported
- Ensure fields are declared as static so these are not exported
- Adjust testsuite-shading to use install_name_tool on MacOS to change the id of the lib. Otherwise the wrong may be used.

Result:

Be able to use multiple shaded versions of the same netty artifact.
@ejona86

This comment has been minimized.

Show comment
Hide comment
@ejona86

ejona86 Aug 20, 2018

Contributor

:-/. Needing to run install_name_tool means we must do the shading on OS X. That's going to be annoying. I really hope no Windows- or Linux-specific issue pops up where we must run commands on those platforms as well.

Contributor

ejona86 commented Aug 20, 2018

:-/. Needing to run install_name_tool means we must do the shading on OS X. That's going to be annoying. I really hope no Windows- or Linux-specific issue pops up where we must run commands on those platforms as well.

@normanmaurer

This comment has been minimized.

Show comment
Hide comment
@normanmaurer

normanmaurer Aug 21, 2018

Member

@ejona86 yeah it is a mess :( I am investigate if there is any way to just do it manually . I guess it’s a matter of rewriting the Mach-o header

Member

normanmaurer commented Aug 21, 2018

@ejona86 yeah it is a mess :( I am investigate if there is any way to just do it manually . I guess it’s a matter of rewriting the Mach-o header

normanmaurer added a commit that referenced this issue Aug 21, 2018

Ensure multiple shaded version of the same netty artifact can be load…
…ed as long as the shaded prefix is different

Motivation:

We should support to load multiple shaded versions of the same netty artifact as netty is often used in multiple dependencies.

This is related to #7272.

Modifications:

- Use -fvisibility=hidden when compiling and use JNIEXPORT for things we really want to have exported
- Ensure fields are declared as static so these are not exported
- Adjust testsuite-shading to use install_name_tool on MacOS to change the id of the lib. Otherwise the wrong may be used.

Result:

Be able to use multiple shaded versions of the same netty artifact.

normanmaurer added a commit that referenced this issue Aug 21, 2018

Ensure multiple shaded version of the same netty artifact can be load…
…ed as long as the shaded prefix is different (#8207)

Motivation:

We should support to load multiple shaded versions of the same netty artifact as netty is often used in multiple dependencies.

This is related to #7272.

Modifications:

- Use -fvisibility=hidden when compiling and use JNIEXPORT for things we really want to have exported
- Ensure fields are declared as static so these are not exported
- Adjust testsuite-shading to use install_name_tool on MacOS to change the id of the lib. Otherwise the wrong may be used.

Result:

Be able to use multiple shaded versions of the same netty artifact.

@normanmaurer normanmaurer self-assigned this Aug 21, 2018

@normanmaurer normanmaurer added this to the 4.1.29.Final milestone Aug 21, 2018

@normanmaurer

This comment has been minimized.

Show comment
Hide comment
@normanmaurer
Member

normanmaurer commented Aug 21, 2018

@ejona86 PTAL #8210 :)

normanmaurer added a commit that referenced this issue Aug 21, 2018

Try to monkey-patch library id when shading is used and we are on Mac…
…OS / OSX.

Motivation:

ea4c315 did ensure we support using multiple versions of the same shaded native library but the user still needed to run install_name_tool -id on MacOS to ensure the ID is unique.
This is kind of error prone and also means that the shading itself would need to be done on MacOS / OSX.

This is related to #7272.

Modifications:

- Monkey patch the shaded native lib on MacOS to ensure the id is unique while unpack it to the tempory location.

Result:

Easier way of using shaded native libs in netty.

normanmaurer added a commit that referenced this issue Aug 21, 2018

Try to monkey-patch library id when shading is used and we are on Mac…
…OS / OSX.

Motivation:

ea4c315 did ensure we support using multiple versions of the same shaded native library but the user still needed to run install_name_tool -id on MacOS to ensure the ID is unique.
This is kind of error prone and also means that the shading itself would need to be done on MacOS / OSX.

This is related to #7272.

Modifications:

- Monkey patch the shaded native lib on MacOS to ensure the id is unique while unpack it to the tempory location.

Result:

Easier way of using shaded native libs in netty.

normanmaurer added a commit that referenced this issue Aug 21, 2018

Try to monkey-patch library id when shading is used and we are on Mac…
…OS / OSX.

Motivation:

ea4c315 did ensure we support using multiple versions of the same shaded native library but the user still needed to run install_name_tool -id on MacOS to ensure the ID is unique.
This is kind of error prone and also means that the shading itself would need to be done on MacOS / OSX.

This is related to #7272.

Modifications:

- Monkey patch the shaded native lib on MacOS to ensure the id is unique while unpack it to the tempory location.

Result:

Easier way of using shaded native libs in netty.

normanmaurer added a commit that referenced this issue Aug 21, 2018

Try to monkey-patch library id when shading is used and we are on Mac…
…OS / OSX.

Motivation:

ea4c315 did ensure we support using multiple versions of the same shaded native library but the user still needed to run install_name_tool -id on MacOS to ensure the ID is unique.
This is kind of error prone and also means that the shading itself would need to be done on MacOS / OSX.

This is related to #7272.

Modifications:

- Monkey patch the shaded native lib on MacOS to ensure the id is unique while unpacking it to the tempory location.

Result:

Easier way of using shaded native libs in netty.

normanmaurer added a commit that referenced this issue Aug 21, 2018

Try to monkey-patch library id when shading is used and we are on Mac…
…OS / OSX.

Motivation:

ea4c315 did ensure we support using multiple versions of the same shaded native library but the user still needed to run install_name_tool -id on MacOS to ensure the ID is unique.
This is kind of error prone and also means that the shading itself would need to be done on MacOS / OSX.

This is related to #7272.

Modifications:

- Monkey patch the shaded native lib on MacOS to ensure the id is unique while unpacking it to the tempory location.

Result:

Easier way of using shaded native libs in netty.

normanmaurer added a commit to netty/netty-tcnative that referenced this issue Aug 23, 2018

Ensure multiple shaded version of the same netty-tcnative artifact ca…
…n be loaded as long as the shaded prefix is different

Motivation:

We should support to load multiple shaded versions of the same netty-tcnative artifact as netty-tcnative is often used in multiple dependencies.

This is related to netty/netty#7272.

Modifications:

- Use -fvisibility=hidden when compiling and use JNIEXPORT for things we really want to have exported
- Ensure fields are declared as static so these are not exported
- Ensure we pass the correct linker flags so functions of the static linked version of Boring|Libre|OpenSSL and APR are not visible

Result:

Be able to use multiple shaded versions of the same netty-tcnative artifact.

normanmaurer added a commit that referenced this issue Aug 23, 2018

Try to monkey-patch library id when shading is used and we are on Mac… (
#8210)

* Try to monkey-patch library id when shading is used and we are on MacOS / OSX.

Motivation:

ea4c315 did ensure we support using multiple versions of the same shaded native library but the user still needed to run install_name_tool -id on MacOS to ensure the ID is unique.
This is kind of error prone and also means that the shading itself would need to be done on MacOS / OSX.

This is related to #7272.

Modifications:

- Monkey patch the shaded native lib on MacOS to ensure the id is unique while unpacking it to the tempory location.

Result:

Easier way of using shaded native libs in netty.
@normanmaurer

This comment has been minimized.

Show comment
Hide comment
@normanmaurer

normanmaurer Aug 23, 2018

Member

Last missing part netty/netty-tcnative#382 :)

Member

normanmaurer commented Aug 23, 2018

Last missing part netty/netty-tcnative#382 :)

@normanmaurer normanmaurer added the defect label Aug 23, 2018

normanmaurer added a commit to netty/netty-tcnative that referenced this issue Aug 23, 2018

Ensure multiple shaded version of the same netty-tcnative artifact ca…
…n be loaded as long as the shaded prefix is different

Motivation:

We should support to load multiple shaded versions of the same netty-tcnative artifact as netty-tcnative is often used in multiple dependencies.

This is related to netty/netty#7272.

Modifications:

- Use -fvisibility=hidden when compiling and use JNIEXPORT for things we really want to have exported
- Ensure fields are declared as static so these are not exported
- Ensure we pass the correct linker flags so functions of the static linked version of Boring|Libre|OpenSSL and APR are not visible

Result:

Be able to use multiple shaded versions of the same netty-tcnative artifact.

normanmaurer added a commit to netty/netty-tcnative that referenced this issue Aug 23, 2018

Ensure multiple shaded version of the same netty-tcnative artifact ca…
…n be loaded as long as the shaded prefix is different (#382)

Motivation:

We should support to load multiple shaded versions of the same netty-tcnative artifact as netty-tcnative is often used in multiple dependencies.

This is related to netty/netty#7272.

Modifications:

- Use -fvisibility=hidden when compiling and use JNIEXPORT for things we really want to have exported
- Ensure fields are declared as static so these are not exported
- Ensure we pass the correct linker flags so functions of the static linked version of Boring|Libre|OpenSSL and APR are not visible

Result:

Be able to use multiple shaded versions of the same netty-tcnative artifact.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment