Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Fix racing on route handler's indexes change#2605

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
franz1981 wants to merge1 commit intovert-x3:4.x
base:4.x
Choose a base branch
Loading
fromfranz1981:4.x_atomic_handler_choice

Conversation

@franz1981
Copy link
Contributor

@franz1981franz1981 commentedMay 6, 2024
edited
Loading

While looking at#2442 I've read some of the historic changes on this part and it's not clear to me yet what the logic and ownership of indexes changes will look like....

For example, reading#739 and#740 I would expect that concurrent updates of indexes, while leaving the indexes thread-unsafe, can causerworsnop/vertx-beans#12, but sadly the original reproducer repo is no longer there :( (ping to@vincentDAO in case he can findhttps://github.com/vincentDAO/vertxbeans-test-heavy somewhere...).

The point is that if concurrent updates can mess up with handlers handling, what we currently d (i.e. read, modify, read again) looks racy and still prone to bugs, although with a smaller chance to make it happen, unless the original bug wasn't at all related racy accesses to the indexes (which instead is my guess, to verify). In this case, my changes can be replaced with a much better#2442 using weaker updates.

Any thoughts?@tsegismont and@vietj or@slinkydeveloper if he remember this distant in time issue?

FYI, this changes just make it "nearly" impossible to break the existing logic, although a racy access on indexes reset (when we set them to 0) can still create some problems because the reset doesn't happen in a single atomic operation (which means that other threads could still observe a context index == 0 and a failure index != 0 or the opposite too!).

I've claimed "nearly", too, because of

if (failure && !hasNextFailureHandler(context) || !failure && !hasNextContextHandler(context)) {
which still uses a racy check (which doesn't advance the indexes, really) - meaning that match can still later fail.
That's why I think is beneficial here to document/explain the expect concurrent behaviour and can be a nice improvement for the next releases (5.x?).

@franz1981franz1981force-pushed the4.x_atomic_handler_choice branch from28ecc98 toacb9aa2CompareMay 6, 2024 07:38
@franz1981franz1981force-pushed the4.x_atomic_handler_choice branch fromacb9aa2 to57ea95dCompareMay 6, 2024 07:42
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@franz1981

[8]ページ先頭

©2009-2025 Movatter.jp