ConcurrentLinkedQueue to unmodifiable list

I have a ConcurrentLinkedQueue where I put some mails in. Now I have a visual page where I want to see what's in the Queue, but of course I do not permit that they alter the Queue in any way.

I put a getter, but instead of the Queue, I return a List. We know that the mail does disappear while we are watching, the use case permits that a refresh of the screen is enough to see the new status.

Code at the moment:

private final ConcurrentLinkedQueue mailsToSend = new ConcurrentLinkedQueue();

public List getMailsToSend() {
    return Collections.unmodifiableList(Lists.newArrayList(mailsToSend.iterator()));
}

I know, it's a very small code fragment and much review isn't there, but if there are better ways to convert it to an unmodifiable list, I'd like to hear it.

The tools in the java.util.concurrent package provide very convenient access to complicated structures that can save a lot of development time. The tools all come with a caveat though - their behaviour when accessed by multiple threads concurrently is deterministic, but not necessarily intuitive. In your code you use:

Lists.newArrayList(mailsToSend.iterator())

This converts the elements of the queue to a List... or, does it? What are the elements of the queue? By the definition of this class, the iterator is guaranteed to return all the elements in the queue at the time the iterator was created, but it may also, if it wants, return elements that were added after the iterator was created, and before it completes. Officially, it is weakly consistent:


they are guaranteed to traverse elements as they existed upon construction exactly once, and may (but are not guaranteed to) reflect any modifications subsequent to construction.

When I use tools in the concurrent library I prefer taking point-in-time copies of the data when I need it, unless I have a good reason not to. In your case, a point-in-time snapshot is what you want.

For this purpose, I prefer the toArray(...) call, which guarantees a copy of the data at the time it was called. Because you cannot tell what size the array will be, you should pas it an empty one in the constructor and it will return a bigger one if needed:

MimeMessage[] messages = mailsToSend.toArray(new MimeMessage[0]);

I would normally return this as a raw array - I like arrays. It also makes it clear that it is a defensive copy of the data.

If you want, though, you can wrap that array in a different format:

return Arrays.asList(messages);

or, even:

return Collections.unmodifiableList(Arrays.asList(mailsToSend.toArray(new MimeMessage[0])));

StackExchange Code Review Q#87732, answer score: 3