Using WeakReference Safely

Question

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 · Accepted Answer · 2016-02-24 15:54:34Z

5

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);

edited Feb 24, 2016 at 15:54

answered Feb 24, 2016 at 15:46

Jon Skeet

1.5m893 gold badges9.3k silver badges9.3k bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

flakes Over a year ago

So if I'm understanding correctly, the GC could perform a cleanup during any line of the execution?

Jon Skeet Over a year ago

@flkes: Yes, if nothing else is keeping it alive.

Alex Cohn Over a year ago

GC cannot destroy the object as soon after get() and while the strong reference that is returned by get() is in scope.

Jon Skeet Over a year ago

@AlexCohn: I'm not quite sure what you're saying, but the point is that in the OP's code they're calling get() and checking whether or not the result is null, but then calling get() again later - and there's no guarantee it will still be non-null at that point.

Alex Cohn Over a year ago

Yes, this if (sub.get() != null) { sub.get().update()); } pattern is no good.

Collectives™ on Stack Overflow

Using WeakReference Safely

1 Answer 1

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related