Skip to content

Fix reference-equality comparison bugs found by Error Prone 2.50.0#4605

Merged
janhoy merged 2 commits into
apache:mainfrom
janhoy:reference-equality-fixes
Jul 3, 2026
Merged

Fix reference-equality comparison bugs found by Error Prone 2.50.0#4605
janhoy merged 2 commits into
apache:mainfrom
janhoy:reference-equality-fixes

Conversation

@janhoy

@janhoy janhoy commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Three latent bugs surfaced by the Error Prone 2.50.0 upgrade in #4604, split out into their own PR per review feedback there:

  • ZkMaintenanceUtils: compared Path objects with == in the single-file-upload case, so the intended branch was never guaranteed to be taken.
  • HttpShardHandler: compared a request-context Boolean with ==; now Boolean.TRUE.equals(...) so any true value enables onlyNrt.
  • ZkShardTermsTest.waitFor: compared boxed values by reference, always burning the 10s timeout for values outside the Integer cache.

Also documents (with a @SuppressWarnings("ReferenceEquality")) that the ClassLoader cache guard in Utils intentionally uses identity comparison.

#4604 will suppress these findings with TODO comments pointing here; once this merges, that branch drops the TODOs on rebase.

- ZkMaintenanceUtils: compared Path objects with == in the single-file
  upload case, so uploading a single file could take the wrong branch
- HttpShardHandler: compared a request-context Boolean with ==; now
  Boolean.TRUE.equals(...) so any true value enables onlyNrt
- ZkShardTermsTest.waitFor: compared boxed values by reference, always
  burning the 10s timeout for values outside the Integer cache
- Utils: document that the ClassLoader cache guard intentionally uses
  identity, and suppress the future ReferenceEquality finding
janhoy added a commit to janhoy/solr that referenced this pull request Jul 3, 2026
Per review feedback, behavior-changing fixes are split into their own PRs
so this PR only carries the mechanical Error Prone 2.50.0 compliance work:

- Reference-equality bug fixes (ZkMaintenanceUtils, HttpShardHandler,
  ZkShardTermsTest) moved to apache#4605
- Charset API modernization (JdkObsolete findings) moved to
  apache#4606

The affected sites revert to the code on main, with @SuppressWarnings and
a TODO comment pointing at the PR that carries the real fix; the
suppressions are dropped when those PRs merge and this branch rebases.
@janhoy janhoy requested review from dsmiley and epugh July 3, 2026 11:35

@dsmiley dsmiley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks. HttpShardHandler one is most impactful of the 3 here.

@janhoy janhoy merged commit 6de3f80 into apache:main Jul 3, 2026
6 checks passed
@janhoy janhoy deleted the reference-equality-fixes branch July 3, 2026 15:12
janhoy added a commit that referenced this pull request Jul 3, 2026
janhoy added a commit to janhoy/solr that referenced this pull request Jul 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants