RFR: 8351230: Collections.synchronizedList returns a list that is not thread-safe [v2]

[Stuart Marks]

On Thu, 15 May 2025 12:17:14 GMT, Per Minborg <pminborg at openjdk.org> wrote:

>> Stuart Marks has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Reversed view of SynchronizedRandomAccessList should also be RandomAccess.
>>   Add tests to ensure RandomAccess is preserved when reversing.
>
> src/java.base/share/classes/java/util/Collections.java line 2782:
> 
>> 2780:             synchronized (mutex) {list.sort(c);}
>> 2781:         }
>> 2782:         public void addFirst(E element) {
> 
> Are these overloads strictly needed as the `default` implementations will delegate to `get` and `set`, which, in turn, are thread safe?

(I think you mean overrides?) Good question. It might be possible to get away without overriding and delegating every method. However, these wrapper classes have historically had bugs that arose from failing to override some methods. This isn't the first time this has happened. Thus I'm inclined to add the overrides even if they aren't strictly necessary. I added this note to the bug report just now:

> It's clear that the getFirst(), getLast(), removeFirst(), and removeLast() default methods must be overridden and synchronized in order to guarantee proper operation. The addFirst() and addLast() methods each resolve to a single call to 'this' and so we could probably get away without overriding them. However, it's possible, though unlikely, that the default method implementations could change, making them unsafe. In addition, it's a bit nicer to the backing list to have all the methods on the wrapper delegate to the corresponding methods of the backing list. This might be helpful if the backing list has some optimizations for particular methods.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/24990#discussion_r2093809289