fix concurrent modification exception in disputes list

This commit is contained in:
woodser 2024-08-05 12:09:17 -04:00
parent d9a3feba8d
commit 3dfaa2fc52
7 changed files with 80 additions and 58 deletions

View File

@ -275,11 +275,13 @@ public class CoreDisputesService {
disputeResult.summaryNotesProperty().get() disputeResult.summaryNotesProperty().get()
); );
synchronized (dispute.getChatMessages()) {
if (reason == DisputeResult.Reason.OPTION_TRADE && if (reason == DisputeResult.Reason.OPTION_TRADE &&
dispute.getChatMessages().size() > 1 && dispute.getChatMessages().size() > 1 &&
dispute.getChatMessages().get(1).isSystemMessage()) { dispute.getChatMessages().get(1).isSystemMessage()) {
textToSign += "\n" + dispute.getChatMessages().get(1).getMessage() + "\n"; textToSign += "\n" + dispute.getChatMessages().get(1).getMessage() + "\n";
} }
}
String summaryText = DisputeSummaryVerification.signAndApply(disputeManager, disputeResult, textToSign); String summaryText = DisputeSummaryVerification.signAndApply(disputeManager, disputeResult, textToSign);

View File

@ -114,6 +114,7 @@ public class DisputeMsgEvents {
// We check at every new message if it might be a message sent after the dispute had been closed. If that is the // We check at every new message if it might be a message sent after the dispute had been closed. If that is the
// case we revert the isClosed flag so that the UI can reopen the dispute and indicate that a new dispute // case we revert the isClosed flag so that the UI can reopen the dispute and indicate that a new dispute
// message arrived. // message arrived.
synchronized (dispute.getChatMessages()) {
ObservableList<ChatMessage> chatMessages = dispute.getChatMessages(); ObservableList<ChatMessage> chatMessages = dispute.getChatMessages();
// If last message is not a result message we re-open as we might have received a new message from the // If last message is not a result message we re-open as we might have received a new message from the
// trader/mediator/arbitrator who has reopened the case // trader/mediator/arbitrator who has reopened the case
@ -127,3 +128,4 @@ public class DisputeMsgEvents {
} }
} }
} }
}

View File

@ -192,6 +192,7 @@ public abstract class SupportManager {
if (ackMessage.getSourceMsgClassName().equals(ChatMessage.class.getSimpleName())) { if (ackMessage.getSourceMsgClassName().equals(ChatMessage.class.getSimpleName())) {
Trade trade = tradeManager.getTrade(ackMessage.getSourceId()); Trade trade = tradeManager.getTrade(ackMessage.getSourceId());
for (Dispute dispute : trade.getDisputes()) { for (Dispute dispute : trade.getDisputes()) {
synchronized (dispute.getChatMessages()) {
for (ChatMessage chatMessage : dispute.getChatMessages()) { for (ChatMessage chatMessage : dispute.getChatMessages()) {
if (chatMessage.getUid().equals(ackMessage.getSourceUid())) { if (chatMessage.getUid().equals(ackMessage.getSourceUid())) {
if (trade.getDisputeState() == Trade.DisputeState.DISPUTE_REQUESTED) { if (trade.getDisputeState() == Trade.DisputeState.DISPUTE_REQUESTED) {
@ -204,6 +205,7 @@ public abstract class SupportManager {
} }
} }
} }
}
} else { } else {
log.warn("Received AckMessage with error state for {} with tradeId={}, sender={}, errorMessage={}", log.warn("Received AckMessage with error state for {} with tradeId={}, sender={}, errorMessage={}",
ackMessage.getSourceMsgClassName(), ackMessage.getSourceId(), ackMessage.getSenderNodeAddress(), ackMessage.getErrorMessage()); ackMessage.getSourceMsgClassName(), ackMessage.getSourceId(), ackMessage.getSenderNodeAddress(), ackMessage.getErrorMessage());
@ -212,6 +214,7 @@ public abstract class SupportManager {
if (ackMessage.getSourceMsgClassName().equals(ChatMessage.class.getSimpleName())) { if (ackMessage.getSourceMsgClassName().equals(ChatMessage.class.getSimpleName())) {
Trade trade = tradeManager.getTrade(ackMessage.getSourceId()); Trade trade = tradeManager.getTrade(ackMessage.getSourceId());
for (Dispute dispute : trade.getDisputes()) { for (Dispute dispute : trade.getDisputes()) {
synchronized (dispute.getChatMessages()) {
for (ChatMessage chatMessage : dispute.getChatMessages()) { for (ChatMessage chatMessage : dispute.getChatMessages()) {
if (chatMessage.getUid().equals(ackMessage.getSourceUid())) { if (chatMessage.getUid().equals(ackMessage.getSourceUid())) {
if (trade.getDisputeState().isCloseRequested()) { if (trade.getDisputeState().isCloseRequested()) {
@ -224,6 +227,7 @@ public abstract class SupportManager {
} }
} }
} }
}
getAllChatMessages(ackMessage.getSourceId()).stream() getAllChatMessages(ackMessage.getSourceId()).stream()
.filter(msg -> msg.getUid().equals(ackMessage.getSourceUid())) .filter(msg -> msg.getUid().equals(ackMessage.getSourceUid()))

View File

@ -353,18 +353,21 @@ public final class Dispute implements NetworkPayload, PersistablePayload {
/////////////////////////////////////////////////////////////////////////////////////////// ///////////////////////////////////////////////////////////////////////////////////////////
public void addAndPersistChatMessage(ChatMessage chatMessage) { public void addAndPersistChatMessage(ChatMessage chatMessage) {
synchronized (chatMessages) {
if (!chatMessages.contains(chatMessage)) { if (!chatMessages.contains(chatMessage)) {
chatMessages.add(chatMessage); chatMessages.add(chatMessage);
} else { } else {
log.error("disputeDirectMessage already exists"); log.error("disputeDirectMessage already exists");
} }
} }
}
public boolean isMediationDispute() { public boolean isMediationDispute() {
return !chatMessages.isEmpty() && chatMessages.get(0).getSupportType() == SupportType.MEDIATION; return !chatMessages.isEmpty() && chatMessages.get(0).getSupportType() == SupportType.MEDIATION;
} }
public boolean removeAllChatMessages() { public boolean removeAllChatMessages() {
synchronized (chatMessages) {
if (chatMessages.size() > 1) { if (chatMessages.size() > 1) {
// removes all chat except the initial guidelines message. // removes all chat except the initial guidelines message.
String firstMessageUid = chatMessages.get(0).getUid(); String firstMessageUid = chatMessages.get(0).getUid();
@ -373,6 +376,7 @@ public final class Dispute implements NetworkPayload, PersistablePayload {
} }
return false; return false;
} }
}
public void maybeClearSensitiveData() { public void maybeClearSensitiveData() {
String change = ""; String change = "";

View File

@ -90,6 +90,7 @@ public abstract class DisputeListService<T extends DisputeList<Dispute>> impleme
/////////////////////////////////////////////////////////////////////////////////////////// ///////////////////////////////////////////////////////////////////////////////////////////
public void cleanupDisputes(@Nullable Consumer<String> closedDisputeHandler) { public void cleanupDisputes(@Nullable Consumer<String> closedDisputeHandler) {
synchronized (disputeList.getObservableList()) {
disputeList.stream().forEach(dispute -> { disputeList.stream().forEach(dispute -> {
String tradeId = dispute.getTradeId(); String tradeId = dispute.getTradeId();
if (dispute.isClosed() && closedDisputeHandler != null) { if (dispute.isClosed() && closedDisputeHandler != null) {
@ -97,6 +98,7 @@ public abstract class DisputeListService<T extends DisputeList<Dispute>> impleme
} }
}); });
} }
}
/////////////////////////////////////////////////////////////////////////////////////////// ///////////////////////////////////////////////////////////////////////////////////////////
@ -130,8 +132,10 @@ public abstract class DisputeListService<T extends DisputeList<Dispute>> impleme
} }
ObservableList<Dispute> getObservableList() { ObservableList<Dispute> getObservableList() {
synchronized (disputeList.getObservableList()) {
return disputeList.getObservableList(); return disputeList.getObservableList();
} }
}
/////////////////////////////////////////////////////////////////////////////////////////// ///////////////////////////////////////////////////////////////////////////////////////////
@ -151,10 +155,12 @@ public abstract class DisputeListService<T extends DisputeList<Dispute>> impleme
isAlerting -> { isAlerting -> {
// We get the event before the list gets updated, so we execute on next frame // We get the event before the list gets updated, so we execute on next frame
UserThread.execute(() -> { UserThread.execute(() -> {
synchronized (disputeList.getObservableList()) {
int numAlerts = (int) disputeList.getList().stream() int numAlerts = (int) disputeList.getList().stream()
.mapToLong(x -> x.getBadgeCountProperty().getValue()) .mapToLong(x -> x.getBadgeCountProperty().getValue())
.sum(); .sum();
numOpenDisputes.set(numAlerts); numOpenDisputes.set(numAlerts);
}
}); });
}); });
disputedTradeIds.add(dispute.getTradeId()); disputedTradeIds.add(dispute.getTradeId());

View File

@ -187,11 +187,13 @@ public abstract class DisputeManager<T extends DisputeList<Dispute>> extends Sup
@Override @Override
public List<ChatMessage> getAllChatMessages(String tradeId) { public List<ChatMessage> getAllChatMessages(String tradeId) {
synchronized (getDisputeList().getObservableList()) {
return getDisputeList().stream() return getDisputeList().stream()
.filter(dispute -> dispute.getTradeId().equals(tradeId)) .filter(dispute -> dispute.getTradeId().equals(tradeId))
.flatMap(dispute -> dispute.getChatMessages().stream()) .flatMap(dispute -> dispute.getChatMessages().stream())
.collect(Collectors.toList()); .collect(Collectors.toList());
} }
}
@Override @Override
public boolean channelOpen(ChatMessage message) { public boolean channelOpen(ChatMessage message) {
@ -239,7 +241,7 @@ public abstract class DisputeManager<T extends DisputeList<Dispute>> extends Sup
} }
public ObservableList<Dispute> getDisputesAsObservableList() { public ObservableList<Dispute> getDisputesAsObservableList() {
synchronized(disputeListService.getDisputeList()) { synchronized(disputeListService.getDisputeList().getObservableList()) {
return disputeListService.getObservableList(); return disputeListService.getObservableList();
} }
} }
@ -249,7 +251,7 @@ public abstract class DisputeManager<T extends DisputeList<Dispute>> extends Sup
} }
protected T getDisputeList() { protected T getDisputeList() {
synchronized(disputeListService.getDisputeList()) { synchronized(disputeListService.getDisputeList().getObservableList()) {
return disputeListService.getDisputeList(); return disputeListService.getDisputeList();
} }
} }
@ -354,7 +356,7 @@ public abstract class DisputeManager<T extends DisputeList<Dispute>> extends Sup
return; return;
} }
synchronized (disputeList) { synchronized (disputeList.getObservableList()) {
if (disputeList.contains(dispute)) { if (disputeList.contains(dispute)) {
String msg = "We got a dispute msg that we have already stored. TradeId = " + dispute.getTradeId() + ", DisputeId = " + dispute.getId(); String msg = "We got a dispute msg that we have already stored. TradeId = " + dispute.getTradeId() + ", DisputeId = " + dispute.getId();
log.warn(msg); log.warn(msg);

View File

@ -287,11 +287,13 @@ public final class ArbitrationManager extends DisputeManager<ArbitrationDisputeL
// set dispute state // set dispute state
cleanupRetryMap(uid); cleanupRetryMap(uid);
synchronized (dispute.getChatMessages()) {
if (!dispute.getChatMessages().contains(chatMessage)) { if (!dispute.getChatMessages().contains(chatMessage)) {
dispute.addAndPersistChatMessage(chatMessage); dispute.addAndPersistChatMessage(chatMessage);
} else { } else {
log.warn("We got a dispute mail msg that we have already stored. TradeId = " + chatMessage.getTradeId()); log.warn("We got a dispute mail msg that we have already stored. TradeId = " + chatMessage.getTradeId());
} }
}
dispute.setIsClosed(); dispute.setIsClosed();
if (dispute.disputeResultProperty().get() != null) { if (dispute.disputeResultProperty().get() != null) {
log.info("We already got a dispute result, indicating the message was resent after updating multisig info. TradeId = " + tradeId); log.info("We already got a dispute result, indicating the message was resent after updating multisig info. TradeId = " + tradeId);