CODEWITHSUNDEEP
javamultithreading
Pentarctagon
Pentarctagon

Reputation: 423

Thread safety of reads from and writes to a variable reference

I've read Oracle's documentation, which states (among a few other things) that:

Reads and writes are atomic for reference variables

So that means, assuming I understand this correctly, that the below code is thread safe without needing volatile, synchronized, or using a Lock class, since the actual assignment of otherHashlist to hashlist is atomic and the assignment from hashlist to tempHashlist is also atomic.

public class SomeClass
{
  private HashMap<Integer, ArrayList<MyObject>> hashlist = null;

  // ...Thread/Runnable logic to periodically call set()...

  private void set()
  {
    HashMap<Integer, ArrayList<MyObject>> otherHashlist = new HashMap<Integer, ArrayList<MyObject>>();

    // ...populate otherHashlist...

    hashlist = otherHashlist;
  }

  public ArrayList<MyObject> get(int i)
  {
    HashMap<Integer, ArrayList<MyObject>> tempHashlist = hashlist;

    return new ArrayList<MyObject>(tempHashlist.get(i));
  }
}

Additionally, hashlist is never accessed in any way outside of get() and set(). set() is also not able to be called, directly or indirectly, by anything outside of the class. The ArrayList returned by get() is new, so operations that modify the ArrayList (set(), remove(), etc) will not affect the original ArrayList in hashlist. I also have not defined any setter methods on MyObject, all of whose member variables are either private final int, private final long, or private final String.

So, my question then, is: Is this code actually thread safe? Or is there some assumption I'm making/angle I'm missing that would make this unsafe?

Upvotes: 0

Views: 186

Answers (2)

Nir Alfasi
Nir Alfasi

Reputation: 53525

Depends on what is the expected behavior...

If every thread has its own instance of SomeClass - it's thread-safe. So let's assume multiple threads have the same instance:

Now say that two threads call set simultaneously, and right after one of them executes the assignment: hashlist = otherHashlist; the other thread is doing the exact same thing (with different content probably). This means that two consecutive calls to get(i) might return different results, which means consistency is not being kept. Further, since threads have local-caching, it could be that some threads will see an older (stale) copy of hashlist.

Is this an accepted behavior ? if yes (which is a bit weird IMO), then you're good.

I highly recommend reading: Java Concurrency in Practice by Brian Goetz, around chapter: "3.5. Safe Publication"

Upvotes: 1

HTNW
HTNW

Reputation: 29193

This isn't thread safe, because although setting the field is safe and won't cause overlapping updates, the changes may not actually be visible by other threads. That is, even if Thread1 sets hashlist to something, Thread2 may not see that change. This is because the JVM is allowed to optimize accesses to hashlist in Thread2, say copying the reference into a register so it doesn't need to do a getfield multiple times. In this case, when Thread1 changes hashlist, Thread2 can't see it, because it is no longer using the field, it is using a local copy of it, which it thinks is the field. This question shows what can happen in cases like this.

The easiest thing to do is to label hashlist volatile, which means changes in one thread are actually seen by others.

Upvotes: 1

Related Questions