Is this proper usage of "safe publication"?

With regards to Safe Publication, consider this piece of code:

public class Test {
    public static void main(String args[]) {
        ThreadB t = new ThreadB();
        t.start();
        t.Grab(new Bag("HI"));
    }
}

class Bag {
    private String item; // <-- no final here

    public String Item() {
        return item;
    }

    public Bag(String item) {
        this.item = item;
    }
}

class ThreadB extends java.lang.Thread {
    private Bag bag;

    public void Grab(Bag bag) {
        this.bag = bag;
    }

    @Override
    public void run() {
        try {
            while (true) {
                if (bag != null) {
                    System.out.println(bag.Item());
                }
            }
        } catch (Exception e) {
            e.printStackTrace();
        }

    }
}

Is it guaranteed that thread B sees the item ("HI") in bag,

Or is it true that thread B may in fact see the item in the bag as null, and will never see the item "HI" in the bag?

Chapter 3.5. Safe Publication in Java Concurrency in Practice contains a very similar example. To cut a long story short: it's not thread safe. ThreadB could see its bag reference as null. Furthermore, ThreadB could see its bag reference as not null, but the referenced Bag's item field could be seen as null by ThreadB.

On my machine sometimes it's an endless loop but I can't reproduce the second scenario when just the Bag.item is null.

Please note that making the Bag.item field final does not make it thread-safe, access to ThreadB.bag also have to be synchronized:

public class ThreadB extends java.lang.Thread {
    private Bag bag;

    public synchronized void setBag(Bag bag) {
        this.bag = bag;
    }

    @Override
    public void run() {
        try {
            while (true) {
                synchronized (this) {
                    if (bag != null) {
                        System.out.println(bag.getItem());
                        break;
                    }
                }
            }
        } catch (Exception e) {
            e.printStackTrace();
        }

    }
}

Or you can use an AtomicReference:

public class ThreadB extends java.lang.Thread {
    private final AtomicReference bag = new AtomicReference();

    public void setBag(final Bag bag) {
        this.bag.set(bag);
    }

    @Override
    public void run() {
        try {
            while (true) {
                Bag bag = this.bag.get();
                if (bag != null) {
                    System.out.println(bag.getItem());
                    break;
                }
            }
        } catch (Exception e) {
            e.printStackTrace();
        }

    }
}

Some other notes:

1,

public String Item() {
    return item;
}

From Code Conventions for the Java Programming Language:


Methods should be verbs, in mixed case with the first letter
lowercase, with the first letter of each internal word capitalized.

So, this method should be called getItem:

public String getItem() {
    return item;
}

2, According to the same document, chapter File Organization, methods should appear after constructors:

public class Bag {
    private String item;

    public Bag(String item) {
        this.item = item;
    }

    public String getItem() {
        return item;
    }
}

StackExchange Code Review Q#6509, answer score: 3