Should my custom async task have synchronized access to what is passed into constructor?

Question

Suppose I have an immutable Model class:

class Model {
    final String id;
    Model(String id) {
        this.id = id;
    }
}

And I have a custom Task class:

class Task extends BlaBlaTask {

    final Model model;

    Task(Model model) {
        this.model = model;
    }

    // runs on a background thread
    void doInBackground() {
        // do smth with model, e.g.:
        String id = model.id;
    }
}

And both Model and Task instances are created on the main UI thread. However doInBackground() is run on another thread. Is this code wrong? Should I add synchronization, e.g. something like this:

class Task extends BlaBlaTask {

    Model model;

    Task(Model model) {
        setModel(model);
    }

    // runs on a background thread
    void doInBackground() {
        // do smth with model, e.g.:
        String id = getModel().id;
    }

    private synchronized void setModel(Model m) {
        model = m;
    }

    private synchronized Model getModel() {
         return model;
    }
}

P.S. I am working on Java 1.4, and the code probably can be run on a multi-core CPU.

score 2 · Accepted Answer · answered Nov 16 '12 at 10:42

2

I'm not familiar anymore with the Java memory model of Java 1.4, but I don't see why you would need synchronization.

If you're starting a thread, then the new thread will see everything you have written before starting the thread.

And if you're passing the task to an existing thread, the publishing mechanism should have all the necessary synchronization in place to make sure that the thread sees everything that has been written before the publication. That shouldn't be the task's job to synchronize anything, it should be the Queue's job (or any other way you use to pass the task from one thread to the other)

answered Nov 16 '12 at 10:42

JB Nizet

678,734
91
1,224
1,255

Thanks, I asked the question being confused by this: http://stackoverflow.com/questions/1919469/question-about-java-concurrency-in-practice-example – Vit Khudenko Nov 16 '12 at 11:13
Could you please look at my another question: http://stackoverflow.com/questions/12934511/possible-concurrency-issue-in-xlet-development ? – Vit Khudenko Nov 19 '12 at 08:08
That's me again.. The answer for http://stackoverflow.com/questions/12934511/possible-concurrency-issue-in-xlet-development has changed. The new point is that visibility still can be an issue even on uni-core CPU. Could you please revise it? – Vit Khudenko Nov 23 '12 at 16:55
AsyncTask API now has an explicit notice on memory observability. – Vit Khudenko Aug 11 '14 at 20:27

score 1 · Answer 2 · answered Nov 16 '12 at 10:40

If you have instantiated both Task (and threrefore its Model as well) before the background thread was started, then there is definitely no synchronization necessary. If the thread is already running and you are just submitting a task to it, and you don't benefit from Java 5's final semantics, there may theoretically be a problem, but it is quite unlikely to actually occur in practice.

score 0 · Answer 3 · answered Aug 11 '14 at 20:23

AsyncTask API now has an explicit notice on this:

Memory observability

AsyncTask guarantees that all callback calls are synchronized in such a way that the following operations are safe without explicit synchronizations.

Set member fields in the constructor or onPreExecute(), and refer to them in doInBackground(Params...).

Set member fields in doInBackground(Params...), and refer to them in onProgressUpdate(Progress...) and onPostExecute(Result).

This is exactly what I was looking for!

Should my custom async task have synchronized access to what is passed into constructor?

3 Answers3

Memory observability