2

While looking at the implementation of android LiveData, I stumbled on the following code snippet

public T getValue() {
        Object data = mData;
        if (data != NOT_SET) {
            return (T) data;
        }
        return null;
    }

Why would one assign a class member to a local variable before returning it? My guess is it is related to the fact that mData is volatile, but I can't fully understand why couldn't we just return mData.

pointer
  • 307
  • 4
  • 12
  • 2
    If this were written `if (mData != NOT_SET) { return (T) mData; }`, then the value of `mData` could be modified by another thread between the `if` and the `return`. – CommonsWare Mar 05 '21 at 13:34

1 Answers1

9

We want to return mData, except if it's equal to NOT_SET because then we want to return null. Imagine if it were naively written like this:

    public T getValue() {
        if (mData != NOT_SET) {
            return (T) mData;
        }
        return null;
    }

The problem is that there is no synchronization at all, so by reading mData twice, we create a race condition. What if the following sequence of events happens?

  1. mData contains some value that's not NOT_SET, so mData != NOT_SET passes and we enter the if block.
  2. Another thread comes along and sets mData = NOT_SET;.
  3. The first thread, now inside the if block, returns (T) mData, which is NOT_SET. But the caller expected null in this case!

In fact this can happen whether or not mData is volatile. The volatile keyword just makes it more likely.

Thomas
  • 174,939
  • 50
  • 355
  • 478
  • the race condition is there with or without `volatile`, just that reading that `mData` into a _local_ field, means that you do not care if `mData` has now changed. You reference is local, un-effected by that change. Besides, instead of doing _two_ volatile reads, you are now doing just one. I think you said the same thing in your answer, right? I am not sure that was your point – Eugene Mar 09 '21 at 21:08