下面的类线程安全性会被破坏吗?我相信它不会被破坏，但我只是想要加倍确定，因为它不容易测试

Question

public class ThreadSafe implements ITaskCompletionListener {

private final Set<String> taskIds = new HashSet<String>();
private final Set<String> successfulIds = new HashSet<String>();
private final Set<String> cancelledIds = new HashSet<String>();
private final Set<String> errorIds = new HashSet<String>();

public ThreadSafe() {

}

// invoked concurrently
@Override
public void onCancel(String pTaskId) {
    remove(pTaskId);
    cancelledIds.add(pTaskId);
}

// invoked concurrently
@Override
public void onError(String pTaskId) {
    remove(pTaskId);
    errorIds.add(pTaskId);
}

// invoked concurrently
@Override
public void onSuccess(String pTaskId) {
    remove(pTaskId);
    successfulIds.add(pTaskId);
}

private void remove(String pTaskId) {
    taskIds.remove(pTaskId);
}

}

Answer 1

这段代码充满了线程安全问题。

HashSet不是线程安全的(它基于非线程安全的HashMap竞争条件，在这种条件下可能会导致infinite loops)。

其次，在taskIds集合中的ID和添加到其他集合中的ID之间没有原子性，因此任务的存在是短暂的。

第三，代码隐含地假设任务状态仅为inProgress -> success|error|cancel，并且没有并发任务执行。如果这不是真的，那么代码将失败。

Answer 2

从HashSet文档中：

请注意，此实现不是同步的。如果多个线程并发访问某个哈希集，并且其中至少有一个线程修改了该哈希集，则必须在外部同步该哈希集

所以，你的代码不是线程安全的。对任何方法的并发访问都可能产生奇怪的结果。

Answer 3

您可以使用线程安全的单个集合，而不是拥有大量的集合并在集合之间传递is。

private final ConcurrentMap<String, State> idState = new ConcurrentHashMap<String, State>();
enum State { TASK, SUCCESS, CANCELLED, ERROR }

public void onSuccess(String taskId) {
    idState.put(taskId, State.SUCCESS);
}

public void onCancelled(String taskId) {
    idState.put(taskId, State.CANCELLED);
}

public void onError(String taskId) {
    idState.put(taskId, State.ERROR);
}

public void remove(String taskId) {
    idState.remove(taskId);
}