Skip to content

keep DualTreeBidiMap consistent when changed through a sub map - #745

Open
rootvector2 wants to merge 1 commit into
apache:masterfrom
rootvector2:dualtreebidimap-sub-map-views
Open

rootvector2 wants to merge 1 commit into
apache:masterfrom
rootvector2:dualtreebidimap-sub-map-views

Conversation

@rootvector2

Copy link
Copy Markdown
Contributor

A DualTreeBidiMap sub map (subMap, headMap, tailMap) decorates an inner bidi map whose normalMap is restricted to the range while its reverseMap is the full map, and the inherited AbstractDualBidiMap code assumes both hold the same mappings. So keySet().clear(), values().clear(), entrySet().clear() and retainAll of an empty collection on a sub map wipe the whole reverse map, put of a value held by a key outside the range leaves two keys with that value, a put rejected for a key out of range has already removed the mapping that held the value, and values().remove() drops the reverse entry of a value outside the range. Found while reading ViewMap, then measured with a randomized differential test of the sub maps against TreeMap: 189778 of 500000 cases left the map and its inverse out of step.

The view now decorates a private ViewBidiMap that overrides clear, containsValue and put for the restricted normalMap, and Values.contains/Values.remove go through the parent map the way View.clear already does. The fix sits in the inner map because putAll, the collection views and nested sub maps all reach it, which the existing ViewMap.clear and ViewMap.containsValue overrides could not cover. The differential test is clean over 1200000 cases after the change, and the new AbstractSortedBidiMapTest cases fail before it and pass after.

  • Read the contribution guidelines for this project.
  • Read the ASF Generative Tooling Guidance if you use Artificial Intelligence (AI).
  • I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute? Claude Code was used to find the bug and to write the fix, the tests and this description.
  • Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.

A sub map view decorates an inner bidi map whose normalMap is restricted
to the range while its reverseMap is the full map. Clearing the keySet,
values or entrySet of a sub map wiped the whole reverse map, put could
leave two keys with one value or drop a mapping before rejecting a key
out of range, and values().remove() could drop the reverse entry of a
value outside the range. The view now decorates a ViewBidiMap that
handles the restricted normalMap.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant