JDK-8278269 : ConcurrentSkipListSet::add works not based on equality
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.util.concurrent
  • Affected Version: 11,17,18
  • Priority: P4
  • Status: Open
  • Resolution: Unresolved
  • OS: generic
  • CPU: generic
  • Submitted: 2021-12-03
  • Updated: 2021-12-10
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
tbdUnresolved
Related Reports
Relates :  
Relates :  
Description
ADDITIONAL SYSTEM INFORMATION :
Debian 5.14.9-2 (2021-10-03) x86_64 GNU/Linux
openjdk version "17.0.1" 2021-10-19
OpenJDK Runtime Environment (build 17.0.1+12-Debian-1)
OpenJDK 64-Bit Server VM (build 17.0.1+12-Debian-1, mixed mode, sharing)


A DESCRIPTION OF THE PROBLEM :

A set is a collection of distinct objects. 
Objects are different if a.equals(b) == false.

import java.util.concurrent.ConcurrentSkipListSet behaves differently since the equality of objects is determined by the compareTo method of contained objects.

The following snippet shows this.
For the sake of the argument the implementation of compareTo always returns 0, which makes them even but not equal.

import java.util.HashSet;
import java.util.concurrent.ConcurrentSkipListSet;

public class NewClass {

	static class E implements Comparable<E> {

		@Override
		public int compareTo(E t) {
			return 0;
		}

	}

	public static void main(String[] args) {

		var set = new HashSet<E>();
		var conc_set = new ConcurrentSkipListSet<E>();

		for (var i = 0; i < 5; i++) {
			set.add(new E());
			conc_set.add(new E());
		}

		System.out.println("set size: " + set.size());
		System.out.println("concurrent set size: " + conc_set.size());

		/**
		* set size: 5
		* concurrent set size: 1
		*/
	}

}

Imho the sets should behave similarly.
The documentation of ConcurrentSkipListSet::add also describes adding wrt equality.

  /**
     * Adds the specified element to this set if it is not already present.
     * More formally, adds the specified element <tt>e</tt> to this set if
     * the set contains no element <tt>e2</tt> such that <tt>e.equals(e2)</tt>.
     * If this set already contains the element, the call leaves the set
     * unchanged and returns <tt>false</tt>.
     */


Maybe I misinterpret sth. here but at least the documentation should note that equality is not used in this kind of set.


STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
See code example. 

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
Set and ConcurrentSkipListSet should behave in similar ways.
ACTUAL -
ConcurrentSkipListSet adds elements only if compareTo != 0 no matter if equals is false.

---------- BEGIN SOURCE ----------
import java.util.HashSet;
import java.util.concurrent.ConcurrentSkipListSet;

public class NewClass {

	static class E implements Comparable<E> {

		@Override
		public int compareTo(E t) {
			return 0;
		}

	}

	public static void main(String[] args) {

		var set = new HashSet<E>();
		var conc_set = new ConcurrentSkipListSet<E>();

		for (var i = 0; i < 5; i++) {
			set.add(new E());
			conc_set.add(new E());
		}

		System.out.println("set size: " + set.size());
		System.out.println("concurrent set size: " + conc_set.size());

		/**
		* set size: 5
		* concurrent set size: 1
		*/
	}

}
---------- END SOURCE ----------

FREQUENCY : always



Comments
Yes, it's been one of the things I've been intending to get to, which is a rework of some of the set-related docs. There are several mentions of equals() in the various SortedSet specs, all of which are incorrect -- they all use the comparison method to determine membership. The "inconsistent with equals" warning is also rather too non-specific; it's confusing, and in one place it mentions behaving "strangely". The Set and Map interfaces really need to deal with the possibility that some implementations might not be based on equals() and instead might use a comparison method (or in the case of IdentityHashMap, and sets derived from that, == instead of equals). [~martin] Let me know if you want to talk further about this. I've had extensive discussions with several folks.
05-12-2021

Someone could try to fix all the spec issues for sorted sets that have comparators inconsistent with equals. It would be nice if there was a way to use inheritDoc to do that rather than copy-paste.
05-12-2021

ConcurrentSkipListSet acts like other sorted set implementations in the JDK. TreeSet has a disclaimer about satisfying the Set interface that is missing from ConcurrentSkipListSet: * <p>Note that the ordering maintained by a set (whether or not an explicit * comparator is provided) must be <i>consistent with equals</i> if it is to * correctly implement the {@code Set} interface. (See {@code Comparable} * or {@code Comparator} for a precise definition of <i>consistent with * equals</i>.) This is so because the {@code Set} interface is defined in * terms of the {@code equals} operation, but a {@code TreeSet} instance * performs all element comparisons using its {@code compareTo} (or * {@code compare}) method, so two elements that are deemed equal by this method * are, from the standpoint of the set, equal. The behavior of a set * <i>is</i> well-defined even if its ordering is inconsistent with equals; it * just fails to obey the general contract of the {@code Set} interface.
05-12-2021

The observations on Windows 10: JDK 11: Failed, the size of ConcurrentSkipListSet is 1 JDK 17: Failed. JDK 18ea+24: Failed. The actual results are correct behaviors because compareTo(E t) always returns 0.
05-12-2021