vrt_vcl: Minor polish of director code #4241
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I had to stop and think twice to realize that I was looking at a simple search loop extracting the result inside its critical section. Isn't the real problem a TOCTOU race between VRT_LookupDirector() and VRT_Assign_Backend()?
Consider the following VCL statement:
set bereq.backend = bereq.backend;
The director's refcount will be decremented before it is incremented. If
that bereq.backend was holding the last reference of a dynamic backend,
the backend is retired and grab a reference afterwards.
While this scenario is highly unlikely, we cannot discard the risk of
assigning a director to a destination that already held a reference to
the same director.
|
#3949 ? |
nigoroll
reviewed
Dec 10, 2024
| @@ -336,6 +336,8 @@ VRT_Assign_Backend(VCL_BACKEND *dst, VCL_BACKEND src) | |||
| AN(dst); | |||
| CHECK_OBJ_ORNULL((*dst), DIRECTOR_MAGIC); | |||
| CHECK_OBJ_ORNULL(src, DIRECTOR_MAGIC); | |||
| if (*dst == src) | |||
There was a problem hiding this comment.
I think you should just push this one to master
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I was in the neighborhood for a quick loop polish and after a couple rounds of confusion I put my finger on a race and an unsafe sequence of events. I think I fixed one and have a suggestion to fix the other one.
It probably needs more homework.