"Java concurrency in practice" - cached thread-safe number factorizer (Listing 2.8)

Question

In the following code (copied from Java Concurrency in Practice Chapter 2, section 2.5, Listing 2.8):

@ThreadSafe
public class CachedFactorizer implements Servlet {
    @GuardedBy("this") private BigInteger lastNumber;
    @GuardedBy("this") private BigInteger[] lastFactors;
    @GuardedBy("this") private long hits;
    @GuardedBy("this") private long cacheHits;

    public synchronized long getHits() { return hits; }

    public synchronized double getCacheHitRatio() {
        return (double) cacheHits / (double) hits;
    }

    public void service(ServletRequest req, ServletResponse resp) {
        BigInteger i = extractFromRequest(req);
        BigInteger[] factors = null;
        synchronized (this) {
            ++hits;
            if (i.equals(lastNumber)) {
                ++cacheHits;
                factors = lastFactors.clone(); // questionable line here
            }
        }
        if (factors == null) {
            factors = factor(i);
            synchronized (this) {
                lastNumber = i;
                lastFactors = factors.clone(); // and here
            }
        }
        encodeIntoResponse(resp, factors);
    }
}

why the factors, lastFactors arrays are cloned? Can't it be simply written as factors = lastFactors; and lastFactors = factors;? Just because the factors is a local variable and it is then passed to encodeIntoResponse, which can modify it?

Hope the question is clear. Thanks.

Answer 1

This is called defensive copying. Arrays are objects as any other, so

 factors = lastFactors

would assing a reference of lastFactos to factors and vice versa. So anyone can overwrite your state outside your control. As an example:

private void filterAndRemove(BigInteger[] arr);
private void encodeIntoResponse(..., BigInteger[] factors) {
   filterAndRemove(factors);
}

With our theoretical assignment filterAndRemove would also affect the original lastFactorials.