On Mon, 19 Oct 2026 09:44:58 GMT, Viktor Klang <[email protected]> wrote:
>> Problem >> new PriorityQueue<>(List.of(new Object())) succeeds today when it should >> throw ClassCastException. initFromCollection relies on heapify() to surface >> incomparable elements, but heapify performs no comparisons on a singleton, >> so the lone element is never validated. Collections with two or more >> non-Comparable elements already throw. >> >> Fix >> In initFromCollection, when the collection holds exactly one element and no >> comparator is in play, self-compare the element before heapifying. This >> mirrors what TreeMap does for its first key (see JDK-5045147). Behavior with >> an explicit comparator is unchanged: non-Comparable elements are still >> accepted when a comparator is provided. >> >> Testing >> Added a copyConstructorSingleton regression test to AddNonComparable >> covering PriorityQueue(Collection) and PriorityQueue(Collection, Comparator) >> with a null comparator, plus the positive cases (Comparable singleton, >> explicit comparator). Verified the new test passes against the patched class >> and the existing queues() test still passes. >> >> >> Please let me know for further questions >> >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > src/java.base/share/classes/java/util/PriorityQueue.java line 316: > >> 314: e.compareTo((E) queue[0]); >> 315: } >> 316: heapify(); > > Is this the optimal place for this check? Would it not be possible to put > this check inside of `initElementsFromCollection`, it would cover more ground > than initFromCollection? Hello @viktorklang-ora, I traced the constructor paths before answering. initFromCollection is reached from the plain-Collection branch of PriorityQueue(Collection) and, via initFromPriorityQueue, from the PriorityQueue-subclass branch. initElementsFromCollection is reached from all of those, plus directly from the SortedSet branch of PriorityQueue(Collection) and from the PriorityQueue(SortedSet) constructor. So you're right: with the check where it is, the SortedSet copy paths bypass it — new PriorityQueue<>(sortedSet) with a single non-Comparable element under natural ordering would still slip through, the same hole one constructor over. Moving the check into initElementsFromCollection, next to the existing len 1 null-check, closes it for every copy path. No behavior change elsewhere: it still only fires when comparator null, the comparator is assigned before the call on every path, and for a singleton heapify is a no-op, so the move is order-independent. I'll move it there and re-run the tests. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32966#discussion_r4315861045
