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

SNOW-1747516: Native libraries are not relocated #1926

Open
laurentgo opened this issue Oct 17, 2024 · 4 comments
Open

SNOW-1747516: Native libraries are not relocated #1926

laurentgo opened this issue Oct 17, 2024 · 4 comments
Assignees
Labels
bug status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector.

Comments

@laurentgo
Copy link
Contributor

Please answer these questions before submitting your issue.
In order to accurately debug the issue this information is required. Thanks!

  1. What version of JDBC driver are you using?

3.19.0

  1. What operating system and processor architecture are you using?

OS X 14.6.1 (arm64)

  1. What version of Java are you using?

Java 11

  1. What did you do?

When using snowflake driver in an application, we experienced crashes we were able to attribute to use of conscrypt native library.
After inspection we realized that snowflake driver packages a relocated version of conscrypt (and grpc) but none of the native libraries bundled with those two artifacts are relocated themselves, and because of it there's a chance that snowflake driver may load a different version of the native library than the one bundled with snowflake

Netty (and by extension grpc-netty-shaded) has a builtin mechanism to detect relocation, and as long as the native libraries use the same relocation pattern (with dots (.) replaced with underscores (_)), netty classes do not need to be rewritten.

Conscrypt on the other hand does not seem to have a builtin mechanism, and along with the native libraries being renamed, the NativeCryptoJni class should be rewritten to change the prefix to search for.

  1. What did you expect to see?

Application crashed (was expecting no crash)

  1. Can you set logging to DEBUG and collect the logs?

No

@laurentgo laurentgo added the bug label Oct 17, 2024
@github-actions github-actions bot changed the title Native libraries are not relocated SNOW-1747516: Native libraries are not relocated Oct 17, 2024
@sfc-gh-dprzybysz
Copy link
Collaborator

Hi @laurentgo,
I have some questions to the issue

  1. Does your application crashes when you are using it via the JDBC API standard interfaces?
  2. Are you using explicitly any class from net.snowflake.client.jdbc.internal.** in your application? Those classes we consider internal and should not be used explicitly.
  3. Have you tried snowflake-jdbc-thin? It does not shade google-cloud-storage (any many others) dependencies and then relocation issues should not happen.
  4. For CLAAssistant check in PR to be fixed I think you should post first the comment that you read and signed CLA, recheck should be the second comment
  5. Please rebase your PR if you still want it to be reviewed, tested and merged

@sfc-gh-dprzybysz sfc-gh-dprzybysz added the status-information_needed Additional information is required from the reporter label Nov 22, 2024
@laurentgo
Copy link
Contributor Author

Hi @laurentgo, I have some questions to the issue

1. Does your application crashes when you are using it via the JDBC API standard interfaces?

Yes, we do use it via the JDBC API

2. Are you using explicitly any class from `net.snowflake.client.jdbc.internal.**` in your application? Those classes we consider internal and should not be used explicitly.

No, we are not using using directly any snowflake internal classes

3. Have you tried snowflake-jdbc-thin? It does not shade google-cloud-storage (any many others) dependencies and then relocation issues should not happen.

I haven't but independently of me using it, I still do think the relocation issue on the main driver should be fixed

4. For CLAAssistant check in PR to be fixed I think you should post first the comment that you read and signed CLA, recheck should be the second comment

👍

5. Please rebase your PR if you still want it to be reviewed, tested and merged

I will

@sfc-gh-dprzybysz
Copy link
Collaborator

Hi @laurentgo
the tests of your PR are running now and we will review then
Please share more details about your setup (you don't have to do this here in OSS ticket, but you can send it to our Support Team e.g. as described in https://community.snowflake.com/s/article/How-To-Submit-a-Support-Case-in-Snowflake-Lodge )

@sfc-gh-dprzybysz sfc-gh-dprzybysz added status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. and removed status-information_needed Additional information is required from the reporter labels Nov 26, 2024
@sfc-gh-dprzybysz
Copy link
Collaborator

Hi @laurentgo
your PR is merged, thank you for the contribution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector.
Projects
None yet
Development

No branches or pull requests

3 participants