Skip to content

Commit

Permalink
Fix BurningManAccountingStore data races
Browse files Browse the repository at this point in the history
Multiple threads read and write to the accounting blocks list causing
data races. Luckily, the LinkedList threw a ConcurrentModificationException
to limit damage. Now, a ReadWriteLock protects the LinkedList against
data races. Multiple threads can read the list at the same time but only
one thread can write to it. Other writing threads wait until it's their
turn.

Fixes bisq-network#6545
  • Loading branch information
alvasw committed Feb 1, 2023
1 parent 742d251 commit dfad3bb
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,66 @@

import com.google.protobuf.Message;

import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.stream.Collectors;

import lombok.Getter;
import lombok.extern.slf4j.Slf4j;

@Slf4j
@Getter
public class BurningManAccountingStore implements PersistableEnvelope {
private final ReentrantReadWriteLock readWriteLock = new ReentrantReadWriteLock();
private final LinkedList<AccountingBlock> blocks = new LinkedList<>();

public BurningManAccountingStore(List<AccountingBlock> blocks) {
this.blocks.addAll(blocks);
}

public void addBlock(AccountingBlock accountingBlock) {
Lock writeLock = readWriteLock.writeLock();
writeLock.lock();
try {
blocks.add(accountingBlock);
} finally {
writeLock.unlock();
}
}

public void purgeLastTenBlocks() {
Lock writeLock = readWriteLock.writeLock();
writeLock.lock();
try {
purgeLast10Blocks();
} finally {
writeLock.unlock();
}
}

public List<AccountingBlock> getBlocks() {
Lock readLock = readWriteLock.readLock();
readLock.lock();
try {
return Collections.unmodifiableList(blocks);
} finally {
readLock.unlock();
}
}

private void purgeLast10Blocks() {
if (blocks.size() <= 10) {
blocks.clear();
return;
}

List<AccountingBlock> purged = new ArrayList<>(blocks.subList(0, blocks.size() - 10));
blocks.clear();
blocks.addAll(purged);
}

public Message toProtoMessage() {
return protobuf.PersistableEnvelope.newBuilder()
.setBurningManAccountingStore(protobuf.BurningManAccountingStore.newBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import java.io.File;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

import lombok.extern.slf4j.Slf4j;
Expand Down Expand Up @@ -61,25 +60,16 @@ public void requestPersistence() {
}

public List<AccountingBlock> getBlocks() {
return Collections.unmodifiableList(store.getBlocks());
return store.getBlocks();
}

public void addBlock(AccountingBlock block) {
store.getBlocks().add(block);
store.addBlock(block);
requestPersistence();
}

public void purgeLastTenBlocks() {
List<AccountingBlock> blocks = store.getBlocks();
if (blocks.size() <= 10) {
blocks.clear();
requestPersistence();
return;
}

List<AccountingBlock> purged = new ArrayList<>(blocks.subList(0, blocks.size() - 10));
blocks.clear();
blocks.addAll(purged);
store.purgeLastTenBlocks();
requestPersistence();
}

Expand Down

0 comments on commit dfad3bb

Please sign in to comment.