views:

154

answers:

1

I have two threads running from a controller class. The first thread receives SMS messages and should continue running as long as the program is in the started state. The other thread is used to calculate the units GPS location.

The controller launches the SMS thread and waits from a text message. If a text message meets certain criteria, the GPS location thread is launched and coordinates are sent back to the controller.

For each thread, I've used the following format:

reader = new Reader(this);
        new Thread(reader).start();

The reader class then uses a reference of the controller so it can call a method in the controller:

public void ReceivedCommand(String address) {
    [..]
}

This method then creates an instance of the GPS thread which itself calls a method from the parent object (thread?) called ReceivedLocation which then sets up the new SMS message (TextMessage object). The problem is that the SMS thread can only return the original sender's address (to reply to) and I need to use the GPS thread so I can set the Payload for the SMS message.

So now I have 2 methods using the same object (TextMessage object), but I want to ensure that the first method (SMS address setter) doesn't change the address while the GPS thread is getting the GPSLocation to set.

Can synchronizing a block within ReceivedCommand():

  • Add the address to the TextMessage object,
  • Run the GPS thread
  • Let the GPS thread call the second method (ReceivedLocation())
  • And let that method change the TextMessage object?
+4  A: 

Firstly, thread creation is expensive. You might be better off using a thread pool (as can be found in the java.util.concurrent package (an ExecutorService) and farming off your work to that

Using synchronized on a shared object will ensure that no two threads can be inside a synchronized block at the same time. However, if I create and start a thread within a synchronized block, I (i.e. the first thread) may have exited the block before the second thread actually starts:

final TextMessage msg = //...
Thread t = new Thread(r);
synchronized (msg) {
    t.start();
} //the other thread is still running and now this thread has not synchronized on the msg

Then your processor r:

Runnable r = new Runnable() {
    public void run() {
        synchronized (msg) { //only any use if readers are alse sync-ed
            msg.setGpsLocation(findGpsLocation(msg)); 
        }
    }
}

As long as the TextMessage object is thread-safe (i.e. field access is synchronized) you should be OK and there is no need to explicitly synchronize in this way.

Note that synchronized is semantically important not just from the perspective of thread-scheduling but also from the fact that it affects data visibility between threads (for example, without synchronization, you cannot be sure that modifications made in one thread will be visible to another).

Modifying my answer to use an ExecutorService:

final TextMessage msg = //...
ExecutorService worker = Executors.newSingleThreadedExecutor();
Future<?> f = worker.submit(r); //the future represents the work

Here, r would look like:

Runnable r = new Runnable() {
    public void run() {
        GpsLocation  loc = findGpsLocation(msg);
        msg.setGpsLocation(loc); //the setter is synchronized
    }
}

It is the setGpsLocation method that should be synchronized (along with the getter and any other field access required by both threads). Note that simply sync-ing field access is not always enough if you require atomicity across fields. For example you may want to update one field conditionally on the value of another - in this case, as long as you synchronize explicitly during the field access, all will be OK:

synchronized (msg) {
    if (msg.getGpsLocation().isIn(AMERICA))
       msg.append(" DUDE!")
}
oxbow_lakes