Skip to content

Declare equals/hashCode consistent with compareTo (CodeQL java/inconsistent-compareto-and-equals)#232

Open
vharseko wants to merge 2 commits into
OpenIdentityPlatform:masterfrom
vharseko:features/codeql-compareto-equals
Open

Declare equals/hashCode consistent with compareTo (CodeQL java/inconsistent-compareto-and-equals)#232
vharseko wants to merge 2 commits into
OpenIdentityPlatform:masterfrom
vharseko:features/codeql-compareto-equals

Conversation

@vharseko

@vharseko vharseko commented Jul 8, 2026

Copy link
Copy Markdown
Member

Summary

Resolves all three open java/inconsistent-compareto-and-equals CodeQL alerts
in the persistit engine. Each flagged class implements Comparable/compareTo
but inherited Object.equals, so equals and compareTo could be
inconsistent:

Class compareTo orders by Used in
BufferPool.BufferHolder volumeId, page Arrays.sort
JournalManager.TransactionMapItem commit / start timestamp TreeSet, sorted List
TreeBuilder.Node tree order, key TreeMap<Node, Node>

Why it is safe

None of these types is currently compared through equals/hashCode:

  • the sorted collections (TreeMap, TreeSet, Arrays.sort,
    Collections.sort) all use compareTo, not equals;
  • the types are never placed in a hash collection keyed by themselves
    (TransactionMapItem is only a value in a HashMap<Long, …>), and
  • there is no contains / indexOf / remove(Object) call that would invoke
    equals on them.

So the inconsistency was latent rather than an active bug, but it is exactly
the trap this rule guards against for future callers.

Fix

For each class, declare equals and hashCode consistent with the existing
compareTo:

  • equals(o) returns true iff o is the same reference or compareTo(o) == 0;
  • hashCode is derived from the same ordering key
    ((volumeId, page); commit-or-start timestamp; the tree instance for a
    Node).

Because nothing relied on the inherited identity equals, behaviour is
unchanged for every existing usage.

Also adds the missing Portions Copyrighted 2026 3A Systems, LLC header line
to JournalManager.java (TreeBuilder.java and BufferPool.java already
carry it).

Testing

mvn -o -pl persistit/core -am compile — BUILD SUCCESS.

…istent-compareto-and-equals)

Three persistit engine classes implemented Comparable/compareTo but inherited
Object.equals, so equals and compareTo could disagree:

  - BufferPool.BufferHolder        (sorted via Arrays.sort by volumeId, page)
  - JournalManager.TransactionMapItem (held in a TreeSet, ordered by timestamp)
  - TreeBuilder.Node               (keyed in a TreeMap, ordered by tree + key)

None of these types is currently compared through equals/hashCode - the sorted
collections (TreeMap, TreeSet, Arrays.sort) all use compareTo, and the types
are never placed in a hash collection keyed by themselves nor looked up via
contains/indexOf/remove(Object). The inconsistency was therefore latent, but
CodeQL flags it and it is a real trap for future callers.

Add equals and hashCode to each class, defined so that two instances are equal
exactly when compareTo returns 0, and hashCode is derived from the same
ordering key. Because nothing relied on the inherited identity equals, this is
behaviour-preserving for all existing usages.

Also add the missing 3A Systems Portions header line to JournalManager.java
(TreeBuilder.java and BufferPool.java already carry it).
@vharseko vharseko added bug java codeql CodeQL static-analysis findings labels Jul 8, 2026
@vharseko vharseko requested a review from maximthomas July 8, 2026 12:55
@vharseko vharseko removed the java label Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug codeql CodeQL static-analysis findings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants