Using WeakReference Safely

Say I have a class Publisher with a list of Subscriber objects stored in a list of WeakReference<>

public interface Subscriber {
    void update();
}

public class Publisher {
    private final List<WeakReference<Subscriber>> subscribers = new CopyOnWriteArrayList<>();

    public void subscribe(final Subscriber subscriber) { 
        subscribers.add(new WeakReference<>(subscriber)); 
    }

    public void publish() { ...

Between a call to Publisher::subscribe and a later call to Publisher::publish, a Subscriber in the weak reference list could have been garbage collected, so I need to check if it is null before using it.

My question is whether or not the code bellow would be safe a safe implementation for publish?

public void publish() { 
    //filter out garbage collected items
    subscribers = subscribers.stream()
            .filter(sub -> sub.get() != null)
            .collect(Collectors.toList());
    //use the remaing objects
    for (final WeakReference<Subscriber> sub : subscribers) {
        sub.get().update());
    }
}

Is it possible that between filtering subscribers and the calls to Subscriber::update that the garbage collector has destroyed another object? Should I be doing a second null check when updating?

    for (final WeakReference<Subscriber> sub : subscribers) {
        if (sub.get() != null) {
            sub.get().update());
        }
    }
Jon Skeet
people
quotationmark

Your suggested second nullity check isn't good enough either, as the first call to get() could return a non-null value, and the second call could return null. I'd suggest:

for (WeakReference<Subscriber> subRef : subscribers) {
    Subscriber sub = subRef.get();
    if (sub != null) {
        sub.update();
    }
}

Or using Java 8's streams (untested):

subscribers
    .stream()
    .map(WeakReference::get)
    .filter(s -> s != null)
    .forEach(Subscriber::update);

people

See more on this question at Stackoverflow