Decline to cache a likely incorrect computation #7485
Merged
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.
Based on my analysis in JENKINS-70206,
Jenkins#getAgentProtocols
is caching a bad value early in the initialization process due to a new race condition introduced by the split ofinstance-identity
to a plugin, even though a correct value could be computed when it is needed later during the test. If this theory is correct, a simple workaround would be to decline to cache the likely incorrect computation, forcing recomputation later when a correct value is computable. Of course a long-term solution would look more like this or explicitly defining the order of initialization within Guice (for example with a constructor parameter), but this should at least stabilize the test suite on Windows in the meantime.Testing done
I've gotten 4 consecutive succesful runs of
jenkins.security.Security218Test
with this PR, which is more than I could get yesterday without this PR. Checking in this change can't hurt, but I'll keep running the build another 2-3 or times and close this PR if a failure ofjenkins.security.Security218Test
demonstrates this PR to be ineffective.Proposed changelog entries
N/A
Proposed upgrade guidelines
N/A
Submitter checklist
@Restricted
or have@since TODO
Javadocs, as appropriate.@Deprecated(since = "TODO")
or@Deprecated(forRemoval = true, since = "TODO")
, if applicable.eval
to ease future introduction of Content Security Policy (CSP) directives (see documentation).Desired reviewers
@mention
Maintainer checklist
Before the changes are marked as
ready-for-merge
:upgrade-guide-needed
label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).lts-candidate
to be considered (see query).