Σάββατο 7 Σεπτεμβρίου 2013

Tip: Is this code thread safe?


import java.util.*;

public class AlertProvider {
  private final List<Alert> alerts = Collections.synchronizedList(new ArrayList<>(10));

  public Collection<Alert> getAlerts() {
    return Collections.unmodifiableCollection(alerts);
  }

  private final static AlertProvider instance = new AlertProvider();

  public static AlertProvider getInstance() {
    return instance;
  }

  public boolean addAlert(Alert alert) {
    return alerts.add(alert);
  }

  public boolean removeAlert(Alert alert) {
    return alerts.remove(alert);
  }

  public void removeAll() {
     Collection<AlerttoRemove = new ArrayList<>();
     for (Alert alert : getAlerts()) {
        if (alert.canBeDeleted()) {
           toRemove.add(alert);
        }
     }
     alerts.removeAll(toRemove);
  }

  public Collection find(final String key) {
     Collection<Alert> alertsFound = new ArrayList<>();
     if (key != null) {
        for (Alert alert : alerts) {
           if (alert.getDescription().contains(key)) {
              alertsFound.add(alert);
           }
        }
     }
     return Collections.unmodifiableCollection(alertsFound);
  }
}


The answer is no. It throws a ConcurrentModificationException. But why? We do have 'secured' our alerts list to be a synchronizedList.

...
...
...
...
...
...
...
...
...

The problem is that while one thread accesses the alerts list, another thread may modify the list. So how can we solve the ConcurrentModificationException?

private final List<Alert> alerts = new CopyOnWriteArrayList<>());

This creates a copy of your list on each modification of its elements. If you don't store many elements to your list you shouldn't worry about performance
Most developers will go away with this change and they are done. Our class is not fully thread safe though. The other lists used inside some of its methods are not thread safe. So either you also make them CopyOnWriteArrayLists or use the synchronized block as shown below.



import java.util.*;


public class AlertProvider {
  private final List<Alert> alerts = new CopyOnWriteArrayList<>();

  public Collection<Alert> getAlerts() {
    return Collections.unmodifiableCollection(alerts);
  }

  private final static AlertProvider instance = new AlertProvider();

  public static AlertProvider getInstance() {
    return instance;
  }

  public boolean addAlert(Alert alert) {
    return alerts.add(alert);
  }

  public boolean removeAlert(Alert alert) {
    return alerts.remove(alert);
  }

  public void removeAll() {
     Collection<AlerttoRemove = new ArrayList<>();
     for (Alert alert : getAlerts()) {
        if (alert.canBeDeleted()) {
          synchronized(toRemove) {
           toRemove.add(alert);
          }
        }
     }
     alerts.removeAll(toRemove);
  }

  public Collection find(final String key) {
     Collection<Alert> alertsFound = new ArrayList<>();
     if (key != null) {
        for (Alert alert : alerts) {
           if (alert.getDescription().contains(key)) {
             synchronized(alertsFound) {
              alertsFound.add(alert);
             }
           }
        }
     }
     return Collections.unmodifiableCollection(alertsFound);
  }
}

Δεν υπάρχουν σχόλια: