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

Add support for virtual threads in smallrye-reactive-messaging #34557

Merged
merged 3 commits into from
Jul 19, 2023

Conversation

anavarr
Copy link
Contributor

@anavarr anavarr commented Jul 5, 2023

merged work from @ozangunalp and @cescoffier to add support for virtual threads in smallrye-reactive-messaging.
When using devmode, using LinkedBlockingDeque for the Aesh console caused deadlocks when using System.out in handlers annotated with @RunOnVirtualThread. The LinkedBlockingDeque was replaced with a ConcurrentLinkedQueue and the deadlock disappeared.
The ReentrantLock used in LinkedBlockingDeque seems to be responsible for the deadlock

@jponge jponge changed the title Add suport for virtual threads in smallrye-reactive-messaging Add support for virtual threads in smallrye-reactive-messaging Jul 6, 2023
@ozangunalp
Copy link
Contributor

Upstream changes in discussion related to this one : smallrye/smallrye-reactive-messaging#2219

@anavarr anavarr force-pushed the smallrye-reactive-messaging branch from 3a53c7c to 99d9827 Compare July 12, 2023 10:48
@ozangunalp
Copy link
Contributor

@anavarr we've released changes to Reactive Messaging in 4.8.0 to control the concurrency better. I'll bump the RM dependency in a separate PR and push updates to this one.

@cescoffier
Copy link
Member

@ozangunalp I let you drive this one home.

…hreads.

Replacing the queue implementation with a lock-free variant resolves the issue
@ozangunalp ozangunalp force-pushed the smallrye-reactive-messaging branch from 99d9827 to de2052d Compare July 17, 2023 07:34
@ozangunalp ozangunalp marked this pull request as ready for review July 17, 2023 07:37
@ozangunalp
Copy link
Contributor

@anavarr as we discussed I pushed changes to this after #34716.
Asking everyone to review it again @jponge @cescoffier @franz1981 @gsmet

Copy link
Member

@jponge jponge left a comment

Choose a reason for hiding this comment

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

LGTM after @cescoffier @franz1981 suggestions

@ozangunalp ozangunalp force-pushed the smallrye-reactive-messaging branch from de2052d to 1b7d001 Compare July 17, 2023 14:13
@github-actions
Copy link

github-actions bot commented Jul 18, 2023

🙈 The PR is closed and the preview is expired.

@quarkus-bot
Copy link

quarkus-bot bot commented Jul 18, 2023

Failing Jobs - Building 1c0b57c

Status Name Step Failures Logs Raw logs
✔️ Maven Tests - JDK 11
Maven Tests - JDK 11 Windows Build ⚠️ Check → Logs Raw logs

@cescoffier cescoffier merged commit 9e0ebaf into quarkusio:main Jul 19, 2023
@quarkus-bot quarkus-bot bot added this to the 3.3 - main milestone Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants