mulitithreading on a shared list

Question

I need to maintain a list of assignments in a multithreaded environment. The executor service will run serveral threads for few minutes, and each thread can add or remove an assignment from the shared list.

I need to prevent a situation where two or more threads are trying to remove the same assignment.

I'm having truble to decide which collection to use, and how to use it. is there any way to do it without wrapping the list inside a syncronyzed block?

From what i read, CopyOnWritearraylist is suitable mainly for reading. Is Collections.synchronizedList is the answer?

thanks.

public class AssignmentsListComponent {

    private List<Assignment> assignmentsList;
    private boolean isClosed;

    public AssignmentsListComponent(List<Assignment> assignmentsList) {
        super();
        this.assignmentsList = assignmentsList;
        this.isClosed = false;
    }

    public void addAssignment(Assignment assignment) throws ClosedAssignmentsListException{
        if(this.isClosed)
            throw new ClosedAssignmentsListException("Cannot add Assignment. List is closed");
        this.assignmentsList.add(assignment);
    }

    public void removeAssignment()throws ClosedAssignmentsListException{
        if(this.isClosed)
            throw new ClosedAssignmentsListException("Cannot add Assignment. List is closed");
        Assignment assignmentToRemove = this.assignmentsList.get(new Random().nextInt(this.assignmentsList.size()-1));
        this.assignmentsList.remove(assignmentToRemove);
        this.isClosed = this.assignmentsList.isEmpty();
    }


}


public class SchedulerService {

    private final static long PERIOD = 1500;//Repeat interval
    private final static long TIME_TO_RUN = 5;

    private Map <Family, AssignmentsListComponent> familyAssignmentsListMap;

    private void initializeFamilyAssignmentsListMap(){
        List<Family> families = HibernateUtil.getAllFamilies();
        for(Family family :families){
            List<Assignment> assignmentsList = new ArrayList<Assignment>();
            AssignmentsListComponent assignmentsListComponent = new AssignmentsListComponent(assignmentsList);
            this.familyAssignmentsListMap.put(family, assignmentsListComponent);
        }

    }

    public void runFamilyMemberThreds(){

        List<Assignment> allAssignments = HibernateUtil.getAllAssignments();
        List<FamilyMember> familyMembers = HibernateUtil.getAllFamilyMembers();
        // create executor to run the threads for all family members
        ScheduledExecutorService executorService = Executors.newScheduledThreadPool(familyMembers.size());

        /**
         * For each of the family members, run in PERIOD intervals, and kill it's task after TIME_TO_RUN in minutes 
         */
        for(FamilyMember familyMember :familyMembers){
            // Execute task every 30 seconds
            final ScheduledFuture<?> handler = executorService.scheduleAtFixedRate(new FamilyMemberThred(familyMember, allAssignments,  familyAssignmentsListMap.get(familyMember.getFamily())),0,PERIOD,TimeUnit.MILLISECONDS);
            Runnable cancel = new Runnable()
            {
                public void run()
                {
                    handler.cancel(true);
                }
            };
            executorService.schedule(cancel,TIME_TO_RUN,TimeUnit.MINUTES);//Cancels the task after 5 minutes
            executorService.shutdown();
        }
    }
}


public class FamilyMemberThred implements Runnable{

        private List<Assignment> allAssignments;
        private FamilyMember familyMember;
        private AssignmentsListComponent assignmentsListComponent;

        public void run() {
            if(isAddAssignment())
                addAssignment();
            else 
                removeAssignment();
        }

        private void addAssignment(){
            Assignment randomAssignmentToAdd = this.allAssignments.get(new Random().nextInt(allAssignments.size()-1));
            try {
                this.assignmentsListComponent.addAssignment(randomAssignmentToAdd);
            } catch (Exception e) {
                logClosedListException(randomAssignmentToAdd);
            }
        }

        private void removeAssignment(){
            try {
                this.assignmentsListComponent.removeAssignment();
            } catch (Exception e) {
                logClosedListException(null);
            }
        }

        private void logClosedListException(Assignment assignment){
            Log log = new Log(familyMember, Action.ADD, assignment, Boolean.FALSE);
            HibernateUtil.saveLog(log);
        }


        /**
         * possible values between 0-2, so if 1 or 2, return true
         */
        private boolean isAddAssignment(){
            return new Random().nextInt(3) > 0;  
        }

    }

Answer 1

Probably, the simplest way to synchronize is to add synchronized keyword for each method of AssignmentsListComponent class. This will make all the methods synchronized, in other word, their execution will be mutable exclusive.

The AssignmentsListComponent's assignmentsList field is passed as a parameter to the constructor. Potentially, the caller may modify the list (it does not in this code snippet but can do that).

Possible solutions for the last issue:

• to create an empty list inside the constructor and assign it to the field. Of course, the caller will not be able to pass the list into the constructor. Which is probably fine. If not, then...
• to copy the parameter, and assign the copy to the field assignmentsList.

Answer 2

CopyOnWriteArrayList is good if you don't plan on having a lot of insertions since it can be inefficient. You seem mostly concerned with removals while another thread is iterating so it should be ok. From another post, the concurrent classes are preferred. You can read here: Difference between CopyOnWriteArrayList and synchronizedList