A rare usage of WeakReference?

I have a class whose instances are initialized and used by underlying flatform.

class MyAttributeConverter implements AttributeConverter<XX, YY> {

    public YY convertToDatabaseColumn(XX attribute) { return null; }

    public XX convertToEntityAttribute(YY dbData) { return null; }
}

Nothing’s wrong and I thought I need to add some static methods for being used as method references.

    private static MyAttributeConverter instance;

    // just a lazy-initialization;
    // no synchronization is required;
    // multiple instantiation is not a problem;
    private static MyAttributeConverter instance() {
        if (instance == null) {
            instance = new MyAttributeConverter();
        }
        return instance;
    }

    // do as MyAttributeConverter::toDatabaseColumn(xx)

    public static YY toDatabaseColumn(XX attribute) {
        return instance().convertToDatabaseColumn(attribute);
    }

    public static XX toEntityAttribute(YY dbData) {
        return instance().convertToEntityAttribute(attribute);
    }

Still nothing seems wrong (I believe) and I don’t like the instance persisted with the class and that’s why I’m trying to do this.

    private static WeakReference<MyAttributeConverter> reference;

    public static <R> R applyInstance(Function<? super MyAttributeConverter, ? extends R> function) {
        MyAttributeConverter referent;
        if (reference == null) {
            referent = new MyAttributeConverter();
            refernce = new WeakReference<>(referent);
            return applyInstance(function);
        }
        referent = reference.get();
        if (referent == null) {
            referent = new MyAttributeConverter();
            refernce = new WeakReference<>(referent);
            return applyInstance(function);
        }
        return function.apply(referent); // @@?
    }

I basically don’t even know how to test this code. And I’m sorry for my questions which each might be somewhat vague.

• Is this a (right/wrong) approach?
• Is there any chance that reference.get() inside the function.apply idiom may be null?
• Is there any chance that there may be some problems such as memory-leak?
• Should I rely on SoftReference rather than WeakReference?

Thank you.

Answer

Note that a method like

// multiple instantiation is not a problem;
private static MyAttributeConverter instance() {
    if (instance == null) {
        instance = new MyAttributeConverter();
    }
    return instance;
}

is not thread safe, as it bears two reads of the instance field; each of them may perceive updates made by other threads or not. This implies that the first read in instance == null may perceive a newer value written by another thread whereas the second in return instance; could evaluate to the previous value, i.e. null. So this method could return null when more than one thread is executing it concurrently. This is a rare corner case, still, this method is not safe. You’d need a local variable to ensure that the test and the return statement use the same value.

// multiple instantiation is not a problem;
private static MyAttributeConverter instance() {
    MyAttributeConverter current = instance;
    if (current == null) {
        instance = current = new MyAttributeConverter();
    }
    return current;
}

This still is only safe when MyAttributeConverter is immutable using only final fields. Otherwise, a thread may return an instance created by another thread in an incompletely constructed state.

You can use the simple way to make it safe without those constraints:

private static final MyAttributeConverter instance = new MyAttributeConverter();

private static MyAttributeConverter instance() {
    return instance;
}

This still is lazy as class initialization only happens on one of the specified triggers, i.e. the first invocation of the method instance().

---

Your usage of WeakReference is subject to the same problems. Further, it’s not clear why you resort to a recursive invocation of your method at two points where you already have the required argument in a local variable.

A correct implementation can be far simpler:

private static WeakReference<MyAttributeConverter> reference;

public static <R> R applyInstance(
    Function<? super MyAttributeConverter, ? extends R> function) {

    WeakReference<MyAttributeConverter> r = reference;
    MyAttributeConverter referent = r != null? r.get(): null;      
    if (referent == null) {
        referent = new MyAttributeConverter();
        reference = new WeakReference<>(referent);
    }
    return function.apply(referent);
}

But before you are going to use it, you should reconsider whether the complicated code is worth the effort. The fact that you are accepting the need to reconstruct the object when it has been garbage collected, even potentially constructing multiple instances on concurrent invocations, suggest that you know that the construction will be cheap. When the construction is cheap, you probably don’t need to cache an instance of it at all.

Just consider

public static <R> R applyInstance(
    Function<? super MyAttributeConverter, ? extends R> function) {

    return function.apply(new MyAttributeConverter());
}

It’s at least worth trying, measuring the application’s performance and comparing it with the other approaches.

On the other hand, it doesn’t look like the instance was occupying a significant amount of memory nor holding non-memory resources. As otherwise, you were more worried about the possibility of multiple instances flying around. So the other variant worth trying and comparing, is the one shown above using a static final field with lazy class initialization and no opportunity to garbage collect that small object.

---

One last clarification. You asked

Is there any chance that reference.get() inside the function.apply idiom may be null?

Since there is no reference.get() invocation inside the evaluation of function.apply, there is no chance that such an invocation may evaluate to null at this point. The function receives a strong reference and since the calling code ensured that this strong reference is not null, it will never become null during the invocation of the apply method.

Generally, the garbage collector will never alter the application state in a way that code using strong references will notice a difference (letting the availability of more memory aside).

But since you asked specifically about reference.get(), a garbage collector may collect an object after its last use, regardless of method executions or local scopes. So the referent could get collected during the execution of the apply method when this method does not use the object anymore. Runtime optimizations may allow this to happen earlier than you might guess by looking at the source code, because what may look like an object use (e.g. a field read) may not use the object at runtime (e.g. because that value is already held in a CPU register, eliminating the need to access the object’s memory). As said, all without altering the method’s behavior.

So a hypothetical reference.get() during the execution of the apply method could in principle evaluate to null, but there is no reason for concern, as said, the behavior of the apply method does not change. The JVM will retain the object’s memory as long as needed for ensuring this correct method execution.

But that explanation was just for completeness. As said, you should not use weak nor soft references for objects not holding expensive resources.