Challenge

Delete every unnecessary volatile

25 minintermediate312 yrs

Edge cases. You have to reason, and two valid fixes differ.

What this teaches

  • Most volatile in real code is redundant, and the ones that matter are missing
  • An existing edge carries plain fields, so the modifier adds nothing
  • volatile on a mutable object protects the reference and not the contents
  • Two reads of one volatile field can straddle a write
  • A constructor that leaks `this` forfeits the final-field guarantee

Starter

Starter.java
import java.util.*;
import java.util.concurrent.*;

/**
 * Challenge: five classes, eleven volatile modifiers, and two real bugs.
 *
 * A team hit a visibility bug once, and the fix that landed was "make shared
 * fields volatile". Most of what follows is that reflex. Your job is to know
 * which ones do work, which are noise, and which two problems the reflex
 * failed to fix.
 *
 * For every field you keep or delete, name the edge in a comment. If you
 * cannot name one, the field needs the modifier. That rule decides nine of
 * the eleven on its own.
 */
public class Starter {

    /* ─────────── 1: handed to a thread at construction ─────────── */

    static class ImportJob implements Runnable {
        private volatile String sourceFile;
        private volatile int batchSize;
        private volatile long imported;

        ImportJob(String sourceFile, int batchSize) {
            this.sourceFile = sourceFile;
            this.batchSize = batchSize;
        }

        public void run() {
            imported = batchSize;
        }

        /** Only ever called after join() on the thread running this job. */
        long imported() {
            return imported;
        }
    }

    /* ─────────── 2: the stop flag ─────────── */

    static class Poller {
        private boolean running = true;
        private volatile long polls;

        void loop() {
            while (running) polls++;
        }

        void stop() {
            running = false;
        }
    }

    /* ─────────── 3: published through a queue ─────────── */

    static class Envelope {
        volatile String recipient;
        volatile String body;

        Envelope(String recipient, String body) {
            this.recipient = recipient;
            this.body = body;
        }
    }

    static class Mailer {
        private final BlockingQueue<Envelope> outbox = new LinkedBlockingQueue<>();

        void send(String to, String body) throws InterruptedException {
            outbox.put(new Envelope(to, body));
        }

        Envelope next() throws InterruptedException {
            return outbox.take();
        }
    }

    /* ─────────── 4: the reloadable settings ─────────── */

    static class Tuning {
        private volatile Map<String, Integer> limits = new HashMap<>();

        void reload(Map<String, Integer> fresh) {
            limits.clear();
            limits.putAll(fresh);
        }

        /** Must never mix an old limit with a new one. */
        String describe() {
            return "requests=" + limits.get("requests") + " burst=" + limits.get("burst");
        }
    }

    /* ─────────── 5: registered on construction ─────────── */

    static class Listener {
        private final String id;
        private final int priority;

        Listener(String id, int priority, Collection<Listener> registry) {
            registry.add(this);
            this.id = id;
            this.priority = priority;
        }

        String id() {
            return id;
        }

        int priority() {
            return priority;
        }
    }

    // TODO 1: for each of the eleven volatile modifiers above, write one line:
    // the edge that already covers the field, or "none — needed".

    // TODO 2: delete every redundant one. Expect to remove most of them.

    // TODO 3: exactly two fields in this file genuinely need an edge and do
    // not have one. Find both. One is a missing volatile; the other cannot be
    // fixed with volatile at all.

    // TODO 4: class 4 has a second defect that survives any modifier —
    // reload() mutates the map every reader is holding, and describe() reads
    // it twice. Fix it the way the entry keeps arriving at, and say why your
    // fix makes both problems unexpressible rather than fixed.

    // TODO 5: class 5 forfeits the final-field guarantee. Name the line, say
    // what another thread can observe, and move it.

    public static void main(String[] args) throws Exception {
        // TODO 6: only ONE of these five can be made to fail reliably in a
        // test. Write that test. Then say what the other four tell you about
        // relying on tests for this class of bug.
        System.out.println("write the test, then delete this line");
    }
}

Run it locally:

cd exercises/java/concurrency/happens-before/02-challenge
javac Starter.java -d /tmp/out && java -cp /tmp/out Starter

Hints

  1. Hint 1

    For each field, name the edge between its write and its read. If there is one, volatile is noise.

  2. Hint 2

    One class has the modifier on the wrong field entirely — the one that needs it has none.

  3. Hint 3

    Look for two reads of the same volatile field in one method.

  4. Hint 4

    One constructor makes the object reachable before it finishes. Find the line that publishes it.

Done when

  • Every redundant volatile is gone, with the edge named in a comment
  • The two fields that genuinely need one have it
  • The straddling read is fixed with a local, not another modifier
  • The leaking constructor no longer publishes an incomplete object

← Back to What is the happens-before relationship?