How to lock two variables written by threadA and read by threadB (Android)

Question

I am working on an Android application that uses two threads, the first (writer) thread is updating two variables, the second (reader) thread uses both in a calculation.
How can I make sure the second thread receives the correct values?
This is my first java locking problem. I have been reading about synchronized, volatile, trylock, but it still remains unclear. For one variable this is 'easy' but do not understand how to do this for two variables.

Details:
The variables are in a shared public class AppState extends Application:

public long appstate.writerVar1;
public int appstate.writerVar2;

In the main activity the threads are created and the appstate is passed to the threads:

writerThread.appstate = appstate;
...
readerThread.appstate = appstate;

The variables then can be accessed by both the writerThread and the readerThread as follows:

writerThread, example:

appstate.writerVar1 = 1234;
appstate.writerVar2 = 5678;

readerThread, example:

if( (appstate.writerVar1 == 9) && (appstate.writerVar2 == 6) ){

It should work like this:

writerThread:
- lock both variables
- update both with new values
- unlock

readerThread:
- lock both variables
- read values
- unlock

Can someone show me the java code to do this? Thanks!

--- Added April 23, 2012 ---

Thanks. Sorry for delayed answer, but read more about this including about mutable objects, http://www.javaranch.com/journal/2003/04/immutable.htm .
I tried to understand how to call this from the writerThread and readerThread, which are created in the main activity, and came up with this code:

writerThread:

private WriterState wsW;
...
var1W = 123;
var2W = 456;
wsW = new WriterState(var1W, var2W);
appstate.setWriterState(wsW);

readerThread:


private WriterState wsR;
...
wsR = appstate.getWriterState();
var1R = wsR.getVar1();
var2R = wsR.getVar2();

I tried this code and it seems to run ok. I can add a new var1W and var2W between reading var1R and var2R.

Now my next questions:
a) is this code ok?
b) assuming the setWriterState code is in a loop, isn't this going to eat all the memory?
c) how can the WriterState class be mutable, I believe this cannot be the case here (you say: If the WriterState class was mutable)

I also saw an example here http://tutorials.jenkov.com/java-concurrency/synchronized.html . They are passing the Counter object to both threads. Isn't something like this possible for this case?

Answer 1

Both synchronized and programatic use of java.util.concurrent.Lock are ok, but I prefer the second one. Assuming that the reader thread needs to do just a simple check on the values, here is a solution:

Edit: This way the reader thread does not get to read the variables. Instead the check the reader is interested in, is done inside the AppState class. Obviously, this will only work if you only have a single thing you need to check for the two variables, and it is known in advance.

public class AppState
{
    private long writerVar1;
    private int writerVar2;

    private ReentrantLock lock = new ReentrantLock();

    public void setValues(long longValue, int intValue)
    {
        lock.lock();
        try
        {
            writerVar1 = longValue;
            writerVar2 = intValue;
        }
        finally
        {
            lock.unlock();
        }
    }

    public boolean isValuesCorrect()
    {
        lock.lock();
        try
        {
            return (writerVar1 == 9) && (writerVar2 == 6);
        }
        finally
        {
            lock.unlock();
        }
    }
//rest of the class body goes here
}

Please note, that both values must be private, and must only be accessed by methods using the same lock, or you don't really have thread safe access.

Answer 2

The key to OO design is encapsulation. And when dealing with multiple threads, it becomes even more important. If those two variables constitute shared state that must be updated and read coherently, then

• this shared state should not be public, because it forces every client of this state to respect a collaboration protocol with the other ones
• this shared state should be encapsulated in an object, which controls access to the state

So, the fields of the AppState object should be private. That's a rule that is true in 99% of the cases. Public fields are a big no-no.

And then all the accesses to the fields should be synchronized (both read and writes). Since those two fields constitute a single coherent piece of information, you should put them into a single class (let's call it WriterState). If this class is mutable, you have to make a copy of the one helf by the shared state each time the getState() and setState() is called, so it's easier and faster to use an immutable class, which can be shared by multiple threads without synchronization.

/**
 * Immutable class: final, contains only final fields wich are themselves immutable.
 */
public final class WriterState {
    private final long var1;
    private final int var2;

    public WriterState(long var1, int var2) {
        this.var1 = var1;
        this.var2 = var2;
    }

    public long getVar1() {
        return this.var1;
    }

    public int getVar2() {
        return this.var2;
    }
}

public class AppState {
    private WriterState writerState;

    public synchronized void setWriterState(WriterState ws) {
        this.writerState = ws;
    }

    public synchronized WriterState getWriterState() {
        return this.writerState;
    }
}

If the WriterState class was mutable, here's what you should do to make it thread-safe:

public class AppState {
    private WriterState writerState;

    public synchronized void setWriterState(WriterState ws) {
        this.writerStatenew WriterState(ws.getVar1(), ws.getVar2());
    }

    public synchronized WriterState getWriterState() {
        return new WriterState(this.writerState.getVar1(), this.writerState.getVar2());
    }
}