How the example from Java Concurrency in practice is thread safe

Question

Below is an example from book Java Concurrency in Practice(Listing 2.8) of a thread safe class.

My question is how the below class is thread safe?

For example if two threads Thread A and Thread B enter into service method of CachedFactorizer. Thread B followed by Thread A. Now if Thread A is executing first synchronized block and Thread B obviously waits for the intrinsic lock of the object. And if Thread B makes it to the first synchronized block before the Thread A makes it to the second synchronized block, it will be viewing a stale value and potentially this condition is known as Race Condition.

So, is my understanding right here? Or am I lacking some basic understanding of concurrency?

@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();
            }
        }
        if (factors == null) {
            factors = factor(i);
            synchronized (this) {
                lastNumber = i;
                lastFactors = factors.clone();
            }
        }
        encodeIntoResponse(resp, factors);
    }
}

Can you be specific about what stale variable you're concerned about. — superfell, Dec 08 '17 at 06:41
All the state variables.hits, factors, lastfactors and cachehits. — Rohit Khurana, Dec 08 '17 at 06:43
You're not the first to question this example, although this one was about using `clone()`: https://stackoverflow.com/questions/12034370/java-concurrency-in-practice-cached-thread-safe-number-factorizer-listing-2 — Kayaman, Dec 08 '17 at 06:51

score 2 · Answer 1 · answered Dec 08 '17 at 07:48

This class is thread safe because all shared variables are accessed in sychronized blocks, and in such situation there is no data race:

"When a program contains two conflicting accesses (§17.4.1) that are not ordered by a happens-before relationship, it is said to contain a data race.https://docs.oracle.com/javase/specs/jls/se9/html/jls-17.html"

The question is if this behaviour is valid from business point of view, eg. if two threads meet after first synchronized blok, they can execute second block in different order.

Yes, if two threads meet after first synchronized block, they can execute second block in different order, which may results in different values for 'lastNumber' and 'lastFactors'. --- That's the point here. — Jason, Jun 11 '21 at 04:21

score 0 · Answer 2 · answered Dec 08 '17 at 06:53

0

The only thing used from within the first block after its exited is factors, which was cloned(), so is unaffected by any other threads getting into the first or second blocks.

answered Dec 08 '17 at 06:53

superfell

18,780
4
59
81

score 0 · Answer 3 · answered May 30 '20 at 22:59

what's the difference if I remove both clone()? eg: factors = lastFactors.clone(); I think it still works.

public void service(ServletRequest req, ServletResponse resp) {

    BigInteger i = extractFromRequest(req);
    BigInteger[] factors = null;
    synchronized (this) {
        ++hits;
        if (i.equals(lastNumber)) {
            ++cacheHits;
            factors = lastFactors; //not cloned
        }
    }
    if (factors == null) {
        factors = factor(i);
        synchronized (this) {
            lastNumber = i;
            lastFactors = factors;//not cloned
        }
    }
    encodeIntoResponse(resp, factors);
}

1.lastFactores only changed to Another array object. not modify the content of the array object. so no need to clone 2. first synchronized is atomic operation of reading two share variables, second synchronized is atomic operation of writing two varaibles. — eiight pan, May 31 '20 at 08:10

How the example from Java Concurrency in practice is thread safe

3 Answers3