Fix data race in IgnoredMailboxMap

Make the 'dataMap' field a ConcurrentHashMap instead of a HashMap, to
prevent a ConcurrentModificationException, which was recently observed
when calling 'IgnoredMailboxMap::toProtoMessage' from the persistence
manager inside the user thread during startup of bisq-desktop. Tracing
through the code, this likely happened during an iteration through the
keyset of 'dataMap' (to check for nulls, inside the proto serialisation
logic during persistence), while simultaneously adding ignored mailbox
messages to the map on a separate thread spawned from the method,
'MailboxMessageService::threadedBatchProcessMailboxEntries'. (The latter
was made asynchronous so as not to block the UI.)

(Since there is a call to 'PersistenceManager::requestPersistence' after
every addition to the ignored mailbox map, there hopefully won't be any
missed entries in the final persisted map, even though the snapshot of
ConcurrentHashMap being iterated through won't be fresh as long as new
entries are simultaneously added.)
This commit is contained in:
Steven Barclay 2024-05-08 18:18:30 +02:00
parent 437f81ff08
commit 23f871a78b
No known key found for this signature in database
GPG Key ID: 9FED6BF1176D500B

View File

@ -17,12 +17,10 @@
package bisq.network.p2p.mailbox; package bisq.network.p2p.mailbox;
import bisq.common.proto.persistable.PersistableEnvelope; import bisq.common.proto.persistable.PersistableEnvelope;
import bisq.common.util.CollectionUtils;
import java.util.HashMap; import java.util.concurrent.ConcurrentHashMap;
import java.util.Map; import java.util.concurrent.ConcurrentMap;
import lombok.EqualsAndHashCode; import lombok.EqualsAndHashCode;
import lombok.Getter; import lombok.Getter;
@ -32,17 +30,17 @@ import lombok.extern.slf4j.Slf4j;
@EqualsAndHashCode @EqualsAndHashCode
public class IgnoredMailboxMap implements PersistableEnvelope { public class IgnoredMailboxMap implements PersistableEnvelope {
@Getter @Getter
private final Map<String, Long> dataMap; private final ConcurrentMap<String, Long> dataMap;
public IgnoredMailboxMap() { public IgnoredMailboxMap() {
this.dataMap = new HashMap<>(); this.dataMap = new ConcurrentHashMap<>();
} }
/////////////////////////////////////////////////////////////////////////////////////////// ///////////////////////////////////////////////////////////////////////////////////////////
// PROTO BUFFER // PROTO BUFFER
/////////////////////////////////////////////////////////////////////////////////////////// ///////////////////////////////////////////////////////////////////////////////////////////
public IgnoredMailboxMap(Map<String, Long> ignored) { private IgnoredMailboxMap(ConcurrentMap<String, Long> ignored) {
this.dataMap = ignored; this.dataMap = ignored;
} }
@ -54,11 +52,7 @@ public class IgnoredMailboxMap implements PersistableEnvelope {
} }
public static IgnoredMailboxMap fromProto(protobuf.IgnoredMailboxMap proto) { public static IgnoredMailboxMap fromProto(protobuf.IgnoredMailboxMap proto) {
return new IgnoredMailboxMap(CollectionUtils.isEmpty(proto.getDataMap()) ? new HashMap<>() : proto.getDataMap()); return new IgnoredMailboxMap(new ConcurrentHashMap<>(proto.getDataMap()));
}
public void putAll(Map<String, Long> map) {
dataMap.putAll(map);
} }
public boolean containsKey(String uid) { public boolean containsKey(String uid) {