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

Issue#11353 - Default virtual thread executor should created named threads #11355

Closed
wants to merge 3 commits into from
Closed

Conversation

danishnawab
Copy link
Contributor

@danishnawab danishnawab commented Jan 30, 2024

Specify a ThreadFactory when setting up the default virtual thread executor so it creates named threads.

(issue #11353)

Tested the reflective logic in probeVirtualThreadExecutor against different JDK versions for compatibility reasons:

  • 19 (preview) ✅
  • 20 (preview) ✅
  • 21 ✅

Likewise, it does not work on the previous versions (JDK 18 and below) as virtual threads were only introduced in JDK 19 (preview).
For older JDKs, the behavior remains the same as before:

  • VirtualThreads#getDefaultVirtualThreadsExecutor reutrns null
  • VirtualThreads#areSupported returns false

@danishnawab danishnawab changed the title default virtual thread executor should created named threads Issue#11353 - Default virtual thread executor should created named threads Jan 30, 2024
@joakime
Copy link
Contributor

joakime commented Jan 30, 2024

Any changes to Jetty 12 must not fail / crash on JDK 17 as well (as that is our minimum JDK version).
I see you put a big red ❌ on JDK 17 in your initial comment, so that makes me nervous, but the CI build is happy and green.
So what did you do?

@joakime joakime linked an issue Jan 30, 2024 that may be closed by this pull request
@danishnawab
Copy link
Contributor Author

Any changes to Jetty 12 must not fail / crash on JDK 17 as well (as that is our minimum JDK version). I see you put a big red ❌ on JDK 17 in your initial comment, so that makes me nervous, but the CI build is happy and green. So what did you do?

This was in reference to Simon's comment here: #11353 (comment)

What I meant to say was that the reflective code fails (as expected) on JDK 17 because there was no support for virtual threads prior to JDK 19 (preview).
The build passes successfully and so do all tests on both JDK 17 and 21.

I can see how the description can be confusing. I will update it accordingly.

@danishnawab
Copy link
Contributor Author

Superseded by #11430

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

The default virtual thread executor should created named threads
3 participants