1
public class Test{
   private MyObj myobj = new MyObj(); //it is not volatile


   public class Updater extends Thred{
      myobje = getNewObjFromDb() ; //not am setting new object
   }

   public MyObj getData(){
    //getting stale date is fine for 
    return myobj;
   }


}

Updated regularly updates myobj
Other classes fetch data using getData
IS this code thread safe without using volatile keyword?
I think yes. Can someone confirm?

Jk1
  • 11,233
  • 9
  • 54
  • 64
user93796
  • 18,749
  • 31
  • 94
  • 150
  • 2
    Recommended book: [Java Concurrency in Practice](http://www.javaconcurrencyinpractice.com/), it explains in detail how this works in Java. – Jesper Oct 23 '12 at 14:36

6 Answers6

7

No, this is not thread safe. (What makes you think it is?)

If you are updating a variable in one thread and reading it from another, you must establish a happens-before relationship between the write and the subsequent read.

In short, this basically means making both the read and write synchronized (on the same monitor), or making the reference volatile.

Without that, there are no guarantees that the reading thread will see the update - and it wouldn't even be as simple as "well, it would either see the old value or the new value". Your reader threads could see some very odd behaviour with the data corruption that would ensue. Look at how lack of synchronization can cause infinite loops, for example (the comments to that article, especially Brian Goetz', are well worth reading):

The moral of the story: whenever mutable data is shared across threads, if you don’t use synchronization properly (which means using a common lock to guard every access to the shared variables, read or write), your program is broken, and broken in ways you probably can’t even enumerate.

Andrzej Doyle
  • 102,507
  • 33
  • 189
  • 228
1

No, it isn't.

Without volatile, calling getData() from a different thread may return a stale cached value.
volatile forces assignments from one thread to be visible on all other threads immediately.

Note that if the object itself is not immutable, you are likely to have other problems.

SLaks
  • 868,454
  • 176
  • 1,908
  • 1,964
  • getting stale data is fine in my case.But dont want to get any exception .Now is it thread safe in this case? – user93796 Oct 23 '12 at 14:32
  • Which exception would you expect? myobj can only be null or another value which are both good return values for MyObj. You need to handle a null return, but yeah, in this sense, it is "threadsafe". – Sebastian Oct 23 '12 at 14:35
  • ConcurrentModifications ones or some concurrancy exceptions.One may try to read data while updator is updating referance.If caller thred gets stale date it is fine with me – user93796 Oct 23 '12 at 14:36
  • @Sebastian I wouldn't be so sure about that. If the reference is non-volatile, does the JLS guarantee that it will be updated atomically? With non-volatile `long` values, an unsynchronized reader can see "tearing", where one word is from the old value and one word is from the new value. And I'm almost certain that the JLS will **not** guarantee that you will get "either the old value or the new value" in this situation. Any behaviour, including a fatal hotspot error, may result. – Andrzej Doyle Oct 23 '12 at 14:39
  • @Sebastian As per Brian Goetz in the link in my answer: "Unless you are well versed in concurrency, your intuition about “what could go wrong” in the absence of synchronization (even on read-only paths) is generally incomplete, and reasoning about the order in which things happen is so tricky that you will almost certainly be wrong." – Andrzej Doyle Oct 23 '12 at 14:40
  • @AndrzejDoyle Am still confused.do i need to use volatile? OR can i do without volatile?Note that i am fine with stale data.Reading theads hsould either get new or old data , – user93796 Oct 23 '12 at 14:44
  • You need to use *appropriate synchronization* for your behaviour. Declaring the field `volatile` means that writes from one thread will be visible to others - this is probably adequate for your needs (though I don't know what they are). There are other ways of handling this (declaring the methods `synchronized`, using an `AtomicReference`, serializing reads and writes to a singleton worker thread) which might make more sense in specific situations. – Andrzej Doyle Oct 23 '12 at 14:48
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/18477/discussion-between-andrzej-doyle-and-user93796) – Andrzej Doyle Oct 23 '12 at 14:49
  • You said "Declaring the field volatile means that writes from one thread will be visible to others". But i dont have the requirement that writes from one thread should be visible to other threads.So i dont think i should use volatile. your comments please – user93796 Oct 23 '12 at 14:50
  • @AndrzejDoyle Yes, you are of course right. I was only thinking of 32-bit references which according to Java are always written atomically. – Sebastian Oct 24 '12 at 08:29
1

You may get a stale reference. You may not get an invalid reference. The reference you get is the value of the reference to an object that the variable points to or pointed to or will point to.

Note that there are no guarantees how much stale the reference may be, but it's still a reference to some object and that object still exists. In other words, writing a reference is atomic (nothing can happen during the write) but not synchronized (it is subject to instruction reordering, thread-local cache et al.).

If you declare the reference as volatile, you create a synchronization point around the variable. Simply speaking, that means that all cache of the accessing thread is flushed (writes are written and reads are forgotten).

The only types that don't get atomic reads/writes are long and double because they are larger than 32-bits on 32-bit machines.

John Dvorak
  • 26,799
  • 13
  • 69
  • 83
1

If MyObj is immutable (all fields are final), you don't need volatile.

irreputable
  • 44,725
  • 9
  • 65
  • 93
0

The big problem with this sort of code is the lazy initialization. Without volatile or synchronized keywords, you could assign a new value to myobj that had not been fully initialized. The Java memory model allows for part of an object construction to be executed after the object constructor has returned. This re-ordering of memory operations is why the memory-barrier is so critical in multi-threaded situations.

Without a memory-barrier limitation, there is no happens-before guarantee so you do not know if the MyObj has been fully constructed. This means that another thread could be using a partially initialized object with unexpected results.

Here are some more details around constructor synchronization:

Constructor synchronization in Java

Community
  • 1
  • 1
Gray
  • 115,027
  • 24
  • 293
  • 354
0

Volatile would work for boolean variables but not for references. Myobj seems to perform like a cached object it could work with an AtomicReference. Since your code extracts the value from the DB I'll let the code stay as is and add the AtomicReference to it.

import java.util.concurrent.atomic.AtomicReference;

    public class AtomicReferenceTest {
        private AtomicReference<MyObj> myobj = new AtomicReference<MyObj>();

        public class Updater extends Thread {

            public void run() {
                MyObj newMyobj = getNewObjFromDb();
                updateMyObj(newMyobj);
            }

            public void updateMyObj(MyObj newMyobj) {
                myobj.compareAndSet(myobj.get(), newMyobj);
            }

             }
        public MyObj getData() {
             return myobj.get();
        }
    }

    class MyObj {
    }
clinton
  • 612
  • 3
  • 6