Serguey Asael Shinder: unmodifiableList gives a view, List.copyOf gives a copy, and the bug is the difference
Two lines that look like the same defensive move, and are not:
return Collections.unmodifiableList(items); // a view of items
return List.copyOf(items); // a snapshot of itemsCollections.unmodifiableList wraps the list you passed. The caller cannot modify it through the wrapper — but you still can, through the original, and the caller sees every one of those changes. That is a view, and the Javadoc says so plainly.
List.copyOf takes the elements and builds a new unmodifiable list. Its contract is equally plain: "If the given Collection is subsequently modified, the returned List will not reflect such modifications." https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/List.html
Where this bites
The classic is a getter that hands out unmodifiableList of a mutable field and is described in the code review as "defensive". It defends against the caller. It does not defend against the object itself, so the caller ends up holding a list that changes between two reads for reasons it cannot see — usually on another thread, usually at the worst moment.
The second bite is the opposite direction. List.copyOf rejects null elements, and that rejection lands at the copy, not where the null was introduced. A collection assembled from a database row with an optional column will throw a NullPointerException in a line that merely copies it, and the stack trace points at the wrong author.
What I do now
- Returning a field:
List.copyOf. If the cost matters, it is because the list is large and hot, and at that point say so with a comment rather than leaving a view behind. - Wrapping something the caller already owns, where they should see later changes —
unmodifiableList, and name the method so it says view. - Accepting a collection into a constructor: copy it there, once. An object that keeps a reference to a caller's mutable list is not immutable, whatever the field declaration says.
The rule underneath
final on a field means the reference cannot be reassigned. It says nothing about the object on the other end of it. Java has no shortage of ways to say "unmodifiable" that mean subtly different things — the question worth asking at every boundary is not "can this be modified" but "who is allowed to see it change".