Take the 2-minute tour ×

Following code to create tasks from a big list which is split into sub lists (List l):

        ExecutorService executor = Executors.newFixedThreadPool(cpu);
        KwArrayDuration duration = new KwArrayDuration();
        for (List<Item> l : partition) {
            Runnable readingTask = new MainReadingTask(l, duration);
        while (!executor.isTerminated()) {
            // Execute all Threads

Sample code of a worker thread:

public class MainReadingTask implements Runnable {

private KwArrayDuration duration;
private List<Item> wis;

    public MainReadingTask(List<Item> wis, KwArrayDuration duration) {
       this.wis = wis;
       this.duration = duration;

    public void run() {
    try {
      for (Item wi : wis) {
        duration.setValueFromItem(wi.getId(), null, cal);
    } catch (IOException e) {


The object of the class KwArrayDuration is used by all threads as a storage class which inserts results from each task in a array list (kwContentArray).

Furthermore the class KwArrayDuration contains some methods to calc some stuff. These methods are invoked by each worker thread:

public synchronized void setValueFromItem(String id, DurationTime duration, Calendar cal) {
   if(duration != null) {
    try {
       this.setDurationValues(id, duration, cal);
    } catch (IOException e) {
   } else {
    int currentDay = getCurrentCalendarDay(cal);
    long time = 0;
    for (int i = currentDay; i < kwContentArray.size(); i++) {
       KwArrayWrapper currentDayKW = kwContentArray.get(i);
       currentDayKW.setValues(wi, String.valueOf(time));
       kwContentArray.set(i, currentDayKW);


Is it really needed to sync setValueFromItem function from shared KwArrayDuration instance? I think yes because results can differ if the scheduler stops e.g. after this line

int currentDay = getCurrentCalendarDay(cal);

and another thread uses this int value for further logic.

share|improve this question

1 Answer 1

up vote 0 down vote accepted

There are two reasons you need to synchronize

  1. If duration is mutable and shared or if cal is shared between other threads, then invoking getCurrentCalendarDay(cal) can result in different results then what was expected upon entering the method. This also applies for duration if it is mutable & shared.

  2. If kwContentArray is shared you also need to synchronize.

Moral of the story, is synchronize access to all shared mutable data.

share|improve this answer
Okay..."cal" is created in constructor of each thread like this: GregorianCalendar cal = new GregorianCalendar(); Still needed to synchronize? –  sk2212 Apr 23 '13 at 7:31
If cal is able to be accessed and modified by more than one thread at a time, then yes. Not the field cal, but cal.set(Calendar.YEAR, 10). Since cal is mutable you run into a data race via setters –  John Vint Apr 23 '13 at 12:16
Hmm, okay...I will create a new question because your answer does not answer my question in all details. I will accept your answer anyway and create more understandable code. –  sk2212 Apr 24 '13 at 9:08

Your Answer


