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 disabling virtual thread context propagation #10881

Merged
merged 2 commits into from
Mar 18, 2024

Conversation

laurit
Copy link
Contributor

@laurit laurit commented Mar 18, 2024

Resolves #10747
Continuation of #10854

@laurit laurit requested a review from a team March 18, 2024 11:14
@laurit laurit merged commit e38093c into open-telemetry:main Mar 18, 2024
49 checks passed
@laurit laurit deleted the virtual-thread branch March 18, 2024 19:32
@Moscagus
Copy link

@laurit the investigation was always based on the latest version of "VirtualThread.java"
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/VirtualThread.java
The signatures are:
private void switchToCarrierThread()
private void switchToVirtualThread(VirtualThread vthread)

But in java 21+13:
https://github.com/openjdk/jdk/blob/jdk-21%2B13/src/java.base/share/classes/java/lang/VirtualThread.java
The signatures are:
private boolean switchToCarrierThread()
private void switchToVirtualThread(VirtualThread vthread, boolean notifyJvmti)

Maybe for this reason the "advices" in "VirtualThreadInstrumentation" are never executed.

@laurit
Copy link
Contributor Author

laurit commented Mar 19, 2024

@laurit the investigation was always based on the latest version of "VirtualThread.java" https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/VirtualThread.java The signatures are: private void switchToCarrierThread() private void switchToVirtualThread(VirtualThread vthread)

But in java 21+13: https://github.com/openjdk/jdk/blob/jdk-21%2B13/src/java.base/share/classes/java/lang/VirtualThread.java The signatures are: private boolean switchToCarrierThread() private void switchToVirtualThread(VirtualThread vthread, boolean notifyJvmti)

Maybe for this reason the "advices" in "VirtualThreadInstrumentation" are never executed.

I don't think so. It was never executed because by default some classes are excluded from instrumentation and I forgot to enable it for the virtual thread class.
I din't realize that 21+13 is an early access version, 2 arg switchToVirtualThread should be handled with #10887

@Moscagus
Copy link

Can you generate a snapshot for me to test in my productive environment ?

@laurit
Copy link
Contributor Author

laurit commented Mar 19, 2024

Can you generate a snapshot for me to test in my productive environment ?

you can find the jar in the artifacts section of the PR https://github.com/open-telemetry/opentelemetry-java-instrumentation/actions/runs/8340418609 or in https://oss.sonatype.org/content/repositories/snapshots/io/opentelemetry/javaagent/opentelemetry-javaagent/2.3.0-SNAPSHOT/ after the PR is merged

@Moscagus
Copy link

After several tests with 1 million virtual threads, I'm pretty sure the solution works.
Thanks @laurit

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.

Posible deadlock with Java 21 Virtual Threads
3 participants