Skip to content

Fix missing unsafe available check#6920

Merged
jack-berg merged 2 commits into
open-telemetry:mainfrom
laurit:unsafe-check
Dec 5, 2024
Merged

Fix missing unsafe available check#6920
jack-berg merged 2 commits into
open-telemetry:mainfrom
laurit:unsafe-check

Conversation

@laurit

@laurit laurit commented Dec 4, 2024

Copy link
Copy Markdown
Contributor

Resolves #6913

@laurit
laurit requested a review from a team as a code owner December 4, 2024 14:58

@trask trask left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it worth (possible?) adding something in CI that runs the tests without unsafe?

@codecov

codecov Bot commented Dec 4, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.09%. Comparing base (d2a0299) to head (2a01ce7).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...emetry/exporter/internal/marshal/UnsafeString.java 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6920      +/-   ##
============================================
- Coverage     90.10%   90.09%   -0.02%     
  Complexity     6594     6594              
============================================
  Files           729      729              
  Lines         19828    19828              
  Branches       1950     1951       +1     
============================================
- Hits          17867    17865       -2     
  Misses         1369     1369              
- Partials        592      594       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@laurit

laurit commented Dec 5, 2024

Copy link
Copy Markdown
Contributor Author

is it worth (possible?) adding something in CI that runs the tests without unsafe?

added a test that uses a custom class loader to hide sun.misc.Unsafe from the tested code

@jack-berg

Copy link
Copy Markdown
Member

Thanks for the fix!

@jack-berg
jack-berg merged commit 3a5222f into open-telemetry:main Dec 5, 2024
@io7m

io7m commented Dec 7, 2024

Copy link
Copy Markdown

Can confirm this fixes the issue. 👍

@trask

trask commented Dec 7, 2024

Copy link
Copy Markdown
Member

is it worth (possible?) adding something in CI that runs the tests without unsafe?

added a test that uses a custom class loader to hide sun.misc.Unsafe from the tested code

ah I see the problem with using something like --sun-misc-unsafe-memory-access=deny is that then gradle/kotlin itself fails 😢

Caused by: java.lang.UnsupportedOperationException: objectFieldOffset
at org.jetbrains.kotlin.com.intellij.util.containers.Unsafe.objectFieldOffset(Unsafe.java:126)

@laurit
laurit deleted the unsafe-check branch December 8, 2024 14:01
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

Successfully merging this pull request may close these issues.

In a modular application, sun.misc.Unsafe is a problem

5 participants