Skip to content

Commit

Permalink
Fix contract violations in ConcurrentReferenceHashMap's EntrySet/Iter…
Browse files Browse the repository at this point in the history
…ator

Closes gh-27454

(cherry picked from commit 208fafa)
  • Loading branch information
jhoeller committed Sep 24, 2021
1 parent 9524ecd commit ebd3517
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 3 deletions.
Expand Up @@ -858,7 +858,7 @@ public boolean contains(@Nullable Object o) {
Reference<K, V> ref = ConcurrentReferenceHashMap.this.getReference(entry.getKey(), Restructure.NEVER);
Entry<K, V> otherEntry = (ref != null ? ref.get() : null);
if (otherEntry != null) {
return ObjectUtils.nullSafeEquals(otherEntry.getValue(), otherEntry.getValue());
return ObjectUtils.nullSafeEquals(entry.getValue(), otherEntry.getValue());
}
}
return false;
Expand Down Expand Up @@ -966,6 +966,7 @@ private void moveToNextSegment() {
public void remove() {
Assert.state(this.last != null, "No element to remove");
ConcurrentReferenceHashMap.this.remove(this.last.getKey());
this.last = null;
}
}

Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2021 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -41,11 +41,13 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;

/**
* Tests for {@link ConcurrentReferenceHashMap}.
*
* @author Phillip Webb
* @author Juergen Hoeller
*/
class ConcurrentReferenceHashMapTests {

Expand Down Expand Up @@ -466,6 +468,7 @@ void shouldRemoveViaEntrySet() {
iterator.next();
iterator.next();
iterator.remove();
assertThatIllegalStateException().isThrownBy(iterator::remove);
iterator.next();
assertThat(iterator.hasNext()).isFalse();
assertThat(this.map).hasSize(2);
Expand All @@ -486,6 +489,26 @@ void shouldSetViaEntrySet() {
assertThat(this.map.get(2)).isEqualTo("2b");
}

@Test
void containsViaEntrySet() {
this.map.put(1, "1");
this.map.put(2, "2");
this.map.put(3, "3");
Set<Map.Entry<Integer, String>> entrySet = this.map.entrySet();
Set<Map.Entry<Integer, String>> copy = new HashMap<>(this.map).entrySet();
copy.forEach(entry -> assertThat(entrySet.contains(entry)).isTrue());
this.map.put(1, "A");
this.map.put(2, "B");
this.map.put(3, "C");
copy.forEach(entry -> assertThat(entrySet.contains(entry)).isFalse());
this.map.put(1, "1");
this.map.put(2, "2");
this.map.put(3, "3");
copy.forEach(entry -> assertThat(entrySet.contains(entry)).isTrue());
entrySet.clear();
copy.forEach(entry -> assertThat(entrySet.contains(entry)).isFalse());
}

@Test
@Disabled("Intended for use during development only")
void shouldBeFasterThanSynchronizedMap() throws InterruptedException {
Expand Down Expand Up @@ -583,7 +606,7 @@ protected int getHash(@Nullable Object o) {
}
// For testing we want more control of the hash
this.supplementalHash = super.getHash(o);
return o == null ? 0 : o.hashCode();
return (o != null ? o.hashCode() : 0);
}

public int getSupplementalHash() {
Expand Down

0 comments on commit ebd3517

Please sign in to comment.