Skip to content

Commit

Permalink
Fix data race in IgnoredMailboxMap
Browse files Browse the repository at this point in the history
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.)
  • Loading branch information
stejbac committed May 8, 2024
1 parent 437f81f commit 23f871a
Showing 1 changed file with 6 additions and 12 deletions.
18 changes: 6 additions & 12 deletions p2p/src/main/java/bisq/network/p2p/mailbox/IgnoredMailboxMap.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,10 @@

package bisq.network.p2p.mailbox;


import bisq.common.proto.persistable.PersistableEnvelope;
import bisq.common.util.CollectionUtils;

import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;

import lombok.EqualsAndHashCode;
import lombok.Getter;
Expand All @@ -32,17 +30,17 @@
@EqualsAndHashCode
public class IgnoredMailboxMap implements PersistableEnvelope {
@Getter
private final Map<String, Long> dataMap;
private final ConcurrentMap<String, Long> dataMap;

public IgnoredMailboxMap() {
this.dataMap = new HashMap<>();
this.dataMap = new ConcurrentHashMap<>();
}

///////////////////////////////////////////////////////////////////////////////////////////
// PROTO BUFFER
///////////////////////////////////////////////////////////////////////////////////////////

public IgnoredMailboxMap(Map<String, Long> ignored) {
private IgnoredMailboxMap(ConcurrentMap<String, Long> ignored) {
this.dataMap = ignored;
}

Expand All @@ -54,11 +52,7 @@ public protobuf.PersistableEnvelope toProtoMessage() {
}

public static IgnoredMailboxMap fromProto(protobuf.IgnoredMailboxMap proto) {
return new IgnoredMailboxMap(CollectionUtils.isEmpty(proto.getDataMap()) ? new HashMap<>() : proto.getDataMap());
}

public void putAll(Map<String, Long> map) {
dataMap.putAll(map);
return new IgnoredMailboxMap(new ConcurrentHashMap<>(proto.getDataMap()));
}

public boolean containsKey(String uid) {
Expand Down

0 comments on commit 23f871a

Please sign in to comment.