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.

Sign up for GitHub

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

Jump to bottom

[UNDERTOW-1758] make state field atomic in HttpServerExchange #1560

Open
wants to merge 2 commits into
base: 2.2.x
Choose a base branch
from
Open

[UNDERTOW-1758] make state field atomic in HttpServerExchange #1560

wants to merge 2 commits into from

Conversation

burov4j
Copy link

@burov4j burov4j commented Feb 22, 2024

@fl4via fl4via added bug fix Contains bug fix(es) next release This PR will be merged before next release or has already been merged (for payload double check) waiting PR update Awaiting PR update(s) from contributor before merging labels Mar 15, 2024
carterkozak
carterkozak reviewed Mar 22, 2024
View reviewed changes
core/src/main/java/io/undertow/server/HttpServerExchange.java
@@ -480,9 +483,9 @@ public HttpServerExchange setRequestURI(final String requestURI) {
public HttpServerExchange setRequestURI(final String requestURI, boolean containsHost) {
this.requestURI = requestURI;
if (containsHost) {
this.state |= FLAG_URI_CONTAINS_HOST;
stateUpdater.accumulateAndGet(this, FLAG_URI_CONTAINS_HOST, (state, flag) -> state | flag);
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps these lambdas could be extracted to static reusable instances -- sometimes lambdas like this in hot paths aren't elided as quickly as one might wish.

Copy link
Author

Choose a reason for hiding this comment

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

Isn't this a premature optimization?

@fl4via fl4via removed the next release This PR will be merged before next release or has already been merged (for payload double check) label Apr 19, 2024
@baranowb baranowb requested a review from ropalka June 24, 2024 10:56
@ropalka ropalka added backport The PR is the result of backporting another PR to a maintainance branch and removed waiting PR update Awaiting PR update(s) from contributor before merging labels Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ropalka ropalka ropalka approved these changes

@carterkozak carterkozak carterkozak left review comments

@fl4via fl4via fl4via requested changes

Assignees
No one assigned
Labels
backport The PR is the result of backporting another PR to a maintainance branch bug fix Contains bug fix(es)
Projects
None yet
Milestone
No milestone
4 participants
@burov4j @ropalka @fl4via @carterkozak