CODEWITHSUNDEEP
javamultithreadingthread-safetysafe-publication
Suryavanshi
Suryavanshi

Reputation: 605

Is this Class thread safe? if not can someone explain a scenario?

I am trying to understand safe publication in case of Effectively Immutable classes. For my class I cannot come up with a scenario in which it would be thread unsafe. Do I need to add some other safe gaurd?

CLARIFICATION: Container elements are thread safe

 public class Container<E> {
    private LinkedList<E> data;

     public Container() {
        this.data = new LinkedList<E>();
    }

    public Container(final Container<E> other) {
        this.data = new LinkedList<E>(other.data);
    }

    public Container<E> add(E e) {
        Container<E> other_cont= new Container<E>(this);
        other_cont.data.add(e);
        return other_cont;
    }

    public Container<E> remove() {
        Container<E> other_cont= new Container<E>(this);
        other_cont.data.remove(0);
        return other_cont;
    }

     public E peek() {
        if(this.data.isEmpty())
            throw new NoSuchElementException("No element to peek at");
        return this.data.get(0);
    }

     public int size() {
        return this.data.size();
    }
 }

Upvotes: 2

Views: 81

Answers (1)

ZhongYu
ZhongYu

Reputation: 19672

This looks fine, as long as a Container is not published unsafely (which we should never do anyway).

But, for the sake of discussion, let's say a Container object is passed through unsafe publication

 static Container shared; // not volatile

 // thread 1
 shared = containerX.add( e );

 // thread 2
 shared.peek();

this is not thread safe; thread 2 can observe corrupt state.

To fix that, we'll need final variable; and all writes should finish before the constructor exit. In your code, you modify other_cont after new Container, which is the problem.

Personally, I would do it this way

 public class Container<E> {
    final private LinkedList<E> data;

    public Container() {
        this.data = new LinkedList<E>();
    }

    Container(LinkedList<E> data) {
        this.data = data;
    }

    public Container<E> add(E e) {

        LinkedList<E> copy = new LinkedList<>(data); 
        copy.add( e );
        return new Container<>(copy);
    }

    ... etc

Rule of thumb for designing an immutable class - all fields must be final, and all writes must be done before constructor exit.

Upvotes: 1

Related Questions