Java Concurrency volatile for reading synchronization for writing

Question

I need to create a class that has a shared-between-threads Object (lets call is SharedObject). The special thing about SharedObject is that it holds a String that will be returned in multithreaded environment, and sometimes the entire SharedObject will be written to by changing field reference to newly created object.

I do not want to make the read and write both synchronised on the same monitor because the write scenario is happening rarely while read scenario is quite common. Therefore I did the following:

public class ObjectHolder {
    private volatile SharedObject sharedObject;

    public String getSharedObjectString() {
        if (!isObjectStillValid()) {
            obtainNewSharedObject()
        }
        return sharedObject.getCommonString()
    }

    public synchronized void obtainNewSharedObject() {
        /* This is in case multiple threads wait on this lock,
           after first one obtains new object the others can just
           use it and should not obtain a new one */
        if(!isObjectStillValid()) {
            sharedObject = new SharedObject(/*some parameters from somewhere*/)
        }
    }
}

From what I have read from documentation and on stackoverflow, the synchronized keyword will assure only one thread can access the synchronised block on the same object instance(therefore write race/multiple unnecessary writes is a non-issue) while volatile keyword on the field reference will assure the reference value is written directly to the main program memory (not cached locally).

Are there any other pitfalls I am missing?

I want to be sure that within synchronized block when sharedObject is written to, the new value of sharedObject is present for any other thread at latest when lock for obtainNewSharedObject() is released. Should this not be guaranteed, I could encounter scenarios of unnecessary writes and replacing correct values which are a big problem for this case.

I know to be absolutely safe I could just make getSharedObjectString() synchronized by itself however as stated previously I do not want to block reading if not needed. This way reading is non-blocking, when a write scenario occurs it is blocking. I should probably mention method isObjectStillValid() is thread independant (entirely SharedObject and System clock based) therefore a valid Thread-free check to be used for write scenarios.

Edit: Please note I could not find a similar topic on stackoverflow, but it may exist. Sorry if that is the case.

Edit2: Thank you for all the comments. Edit because apparently I cannot upvote yet (I can, but it does not show). While my solution is functional as long as isObjectStillValid is thread-safe, it can suffer from decreased performance due to multiple accesses to volatile field. I will improve it most likely using the upgraded double-checked locking solution. I will also in-depth analyse all the other possibilities mentioned here.

Answer 1

Why don't you use AtomicReference. It uses optimistic locking, meaning that no actual thread locking is involved. Internally it uses Compare and Swap. If you look at the implementation it uses volatile in its implementation and I would trust Doug Lea to implement it correctly :)

Apart from this, there many more ways for synchronization between lot of readers and some writers - ReadWriteLock