-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Vert.x event-loop threads start as "vert.x-eventloop-thread-" #13143
Vert.x event-loop threads start as "vert.x-eventloop-thread-" #13143
Conversation
This typo causes a bug where blocking checks fail when run on a Vert.x event-loop.
This is a stupid bug, sorry for the mistake. |
/cc @cescoffier |
@@ -36,7 +36,7 @@ public boolean getAsBoolean() { | |||
* calling from a Vert.x event-loop context / thread. | |||
*/ | |||
String threadName = Thread.currentThread().getName(); | |||
return !threadName.contains("vertx-eventloop-thread-"); | |||
return !threadName.startsWith("vert.x-eventloop-thread-"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't we have a constant in Vert.x or an API to test that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, because this would add dependency from this extension to Vert.x core. I had initially tried writing the test with Vert.x APIs, but that didn't work here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this naming scheme has been consistent over the years and is not changing anytime soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to avoid having an explicit dependency on Vert.x. I believe there is a comment about that.
That being said, we can create a constant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a constant
a2a2a74
to
5332dc7
Compare
@cescoffier I let you check and merge this one. Thanks! |
This typo causes a bug where blocking checks fail when run on a Vert.x event-loop.