Skip to content

Commit 137e04f

Browse files
HashEngineeringHashEngineering
authored
fix: improve refresh unused keys and other things to support reliable shutdown (#296)
* fix: improve refreshUnusedKeys for locks * fix: improve getMixingProgress algorithm accuracy * fix: add test for improved getMixingProgress * fix: PeerGroup: only start downloading from new peer if running * fix: improve shutdown/close operations * don't remove peer listeners, that will be handled by PeerGroup * feat (examples): Add DumpMasternodeList * fix: restore removal of peergroup listeners * fix: restore removal of peergroup listeners in CoinJoinManager * fix: add check for null blockChain * fix: improve DumpMasternodeList processing of mnlistdiff message * fix: set some logging to debug in CoinJoinManager * tests: fix LargeCoinJoinWalletTest * chore: update tests CI to support MacOS latest
1 parent 5d9b0c9 commit 137e04f

17 files changed

Lines changed: 318 additions & 44 deletions

File tree

‎.github/workflows/tests.yml‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,16 @@ jobs:
1414
fail-fast: false
1515
name: JAVA ${{ matrix.distribution }} ${{ matrix.java }} OS ${{ matrix.os }} Gradle ${{ matrix.gradle }}
1616
steps:
17-
- uses: actions/checkout@v1
17+
- uses: actions/checkout@v4
18+
with:
19+
submodules: recursive
1820
- name: Set up JDK
19-
uses: actions/setup-java@v1
21+
uses: actions/setup-java@v4
2022
with:
23+
distribution: 'temurin'
2124
java-version: ${{ matrix.java }}
2225
- name: Build bls library
2326
run: |
24-
git submodule update --init --recursive
2527
cd contrib/dashj-bls
2628
git apply catch_changes.patch
2729
mvn package -DskipTests -Dmaven.javadoc.skip=true

‎core/src/main/java/org/bitcoinj/coinjoin/utils/CoinJoinManager.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,8 @@ public void close() {
296296
if (masternodeGroup != null) {
297297
masternodeGroup.removePreMessageReceivedEventListener(preMessageReceivedEventListener);
298298
}
299-
// Ensure executor is shut down
299+
300+
// Shut down the executor before nulling fields — queued tasks may still reference them.
300301
ExecutorService execToStop = null;
301302
lock.lock();
302303
try {
@@ -310,6 +311,8 @@ public void close() {
310311
if (execToStop != null) {
311312
execToStop.shutdown();
312313
}
314+
blockChain = null;
315+
peerGroup = null;
313316
}
314317

315318
public boolean isMasternodeOrDisconnectRequested(MasternodeAddress address) {
@@ -547,12 +550,12 @@ public void processTransaction(Transaction tx) {
547550
if(!alreadyHave(item)) {
548551
getdata.addItem(item);
549552
} else {
550-
log.info("coinjoin: DSQUEUE: already has {}", item.hash);
553+
log.debug("coinjoin: DSQUEUE: already has {}", item.hash);
551554
}
552555
}
553556
if (!getdata.getItems().isEmpty()) {
554557
// This will cause us to receive a bunch of block or tx messages.
555-
log.info(COINJOIN_EXTRA, "coinjoin: DSQUEUE: requesting {} dsq messages", getdata.getItems().size());
558+
log.debug(COINJOIN_EXTRA, "coinjoin: DSQUEUE: requesting {} dsq messages", getdata.getItems().size());
556559
getdata.getItems().forEach(
557560
inventoryItem -> log.info(COINJOIN_EXTRA, "getdata: {}", inventoryItem.hash));
558561
peer.sendMessage(getdata);

‎core/src/main/java/org/bitcoinj/core/AbstractManager.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,9 @@ public void setFilename(String filename) {
308308
autosaveToFile(new File(filename), DELAY_TIME, TimeUnit.MILLISECONDS, null);
309309
}
310310

311+
/**
312+
* Typically called when DashSystem is shutting down.
313+
*/
311314
public void close() {
312315
if (vFileManager != null) {
313316
shutdownAutosaveAndWait();

‎core/src/main/java/org/bitcoinj/core/DualBlockChain.java‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@
1616

1717
package org.bitcoinj.core;
1818

19+
import org.bitcoinj.core.listeners.DownloadProgressTracker;
1920
import org.bitcoinj.store.BlockStoreException;
21+
import org.bitcoinj.utils.Threading;
2022

2123
import javax.annotation.Nullable;
2224

@@ -105,4 +107,56 @@ public StoredBlock getBlock(int height) {
105107
public StoredBlock getChainHead() {
106108
return getLongestChain().chainHead;
107109
}
110+
111+
PeerGroup peerGroup;
112+
MyDownloadProgressTracker downloadProgressTracker;
113+
114+
private static class MyDownloadProgressTracker extends DownloadProgressTracker {
115+
volatile boolean headerDownloadCompleted = false;
116+
volatile boolean blockDownloadCompleted = false;
117+
MyDownloadProgressTracker(boolean preprocessingBeforeBlocks) {
118+
super(preprocessingBeforeBlocks);
119+
}
120+
121+
@Override
122+
public void doneHeaderDownload() {
123+
super.doneHeaderDownload();
124+
headerDownloadCompleted = true;
125+
}
126+
127+
@Override
128+
protected void doneDownload() {
129+
super.doneDownload();
130+
blockDownloadCompleted = true;
131+
}
132+
}
133+
134+
public void setPeerGroup(PeerGroup peerGroup, MasternodeSync masternodeSync) {
135+
close();
136+
if (peerGroup != null) {
137+
this.peerGroup = peerGroup;
138+
downloadProgressTracker = new MyDownloadProgressTracker(masternodeSync.hasSyncFlag(MasternodeSync.SYNC_FLAGS.SYNC_BLOCKS_AFTER_PREPROCESSING));
139+
peerGroup.addHeadersDownloadedEventListener(Threading.USER_THREAD, downloadProgressTracker);
140+
peerGroup.addHeadersDownloadStartedEventListener(Threading.USER_THREAD, downloadProgressTracker);
141+
peerGroup.addBlocksDownloadedEventListener(Threading.USER_THREAD, downloadProgressTracker);
142+
peerGroup.addChainDownloadStartedEventListener(Threading.USER_THREAD, downloadProgressTracker);
143+
peerGroup.addMasternodeListDownloadListener(Threading.USER_THREAD, downloadProgressTracker);
144+
}
145+
}
146+
147+
public boolean isInitialHeaderSyncComplete() {
148+
return downloadProgressTracker != null && (downloadProgressTracker.headerDownloadCompleted || downloadProgressTracker.blockDownloadCompleted);
149+
}
150+
151+
public void close() {
152+
if (peerGroup != null && downloadProgressTracker != null) {
153+
peerGroup.removeHeadersDownloadedEventListener(downloadProgressTracker);
154+
peerGroup.removeHeadersDownloadStartedEventListener(downloadProgressTracker);
155+
peerGroup.removeBlocksDownloadedEventListener(downloadProgressTracker);
156+
peerGroup.removeChainDownloadStartedEventListener(downloadProgressTracker);
157+
peerGroup.removeMasternodeListDownloadedListener(downloadProgressTracker);
158+
}
159+
peerGroup = null;
160+
downloadProgressTracker = null;
161+
}
108162
}

‎core/src/main/java/org/bitcoinj/core/MasternodeSync.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,8 @@ public void close() {
126126
if (peerGroup != null) {
127127
peerGroup.removePreMessageReceivedEventListener(preMessageReceivedEventListener);
128128
}
129+
peerGroup = null;
130+
blockChain = null;
129131
}
130132

131133
public MasternodeSync(Context context, boolean isLiteMode, boolean allowInstantSendInLiteMode) {

‎core/src/main/java/org/bitcoinj/core/PeerGroup.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2044,7 +2044,7 @@ protected void handlePeerDeath(final Peer peer, @Nullable Throwable exception) {
20442044
final Peer newDownloadPeer = selectDownloadPeer(peers);
20452045
if (newDownloadPeer != null) {
20462046
setDownloadPeer(newDownloadPeer);
2047-
if (downloadListener != null) {
2047+
if (downloadListener != null && vRunning) {
20482048
startBlockChainDownloadFromPeer(newDownloadPeer);
20492049
}
20502050
}

‎core/src/main/java/org/bitcoinj/core/SporkManager.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ private static void makeSporkDefinition(SporkId sporkId, long defaultValue) {
7272
@GuardedBy("lock") private final HashMap<SporkId, Long> mapSporksCachedValues;
7373
@GuardedBy("lock") private final HashSet<KeyId> setSporkPubKeyIds = new HashSet<>();
7474

75+
private PeerGroup peerGroup;
7576
private AbstractBlockChain blockChain;
7677
private MasternodeSync masternodeSync;
7778
private final Context context;
@@ -91,6 +92,7 @@ public SporkManager(Context context)
9192
public void setBlockChain(AbstractBlockChain blockChain, @Nullable PeerGroup peerGroup, MasternodeSync masternodeSync) {
9293
this.blockChain = blockChain;
9394
this.masternodeSync = masternodeSync;
95+
this.peerGroup = peerGroup;
9496
if (peerGroup != null) {
9597
peerGroup.addConnectedEventListener(peerConnectedEventListener);
9698
peerGroup.addPreMessageReceivedEventListener(SAME_THREAD, preMessageReceivedEventListener);
@@ -102,18 +104,23 @@ public void clear() {
102104
mapSporksByHash.clear();
103105
}
104106

105-
public void close(PeerGroup peerGroup) {
107+
public void close() {
106108
if (peerGroup != null) {
107109
peerGroup.removeConnectedEventListener(peerConnectedEventListener);
108110
peerGroup.removePreMessageReceivedEventListener(preMessageReceivedEventListener);
109111
}
112+
peerGroup = null;
113+
blockChain = null;
114+
masternodeSync = null;
110115
}
111116

112117
void processSpork(Peer from, SporkMessage spork) {
113118
// if (context.isLiteMode() && !context.allowInstantXinLiteMode()) {
114119
// return; //disable all darksend/masternode related functionality
115120
// }
116121

122+
if (blockChain == null) return; // closed during shutdown
123+
117124
Sha256Hash hash = spork.getHash();
118125
String logMessage = String.format("SPORK -- hash: %s id: %d (%s) value: %10d bestHeight: %d peer=%s:%d",
119126
hash, spork.getSporkId().value, String.format("%1$35s", spork.getSporkId().name()),

‎core/src/main/java/org/bitcoinj/evolution/AbstractQuorumState.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -554,7 +554,7 @@ public void removeEventListeners(AbstractBlockChain blockChain, PeerGroup peerGr
554554
blockChain.removeNewBestBlockListener(newBestBlockListener);
555555
blockChain.removeReorganizeListener(reorganizeListener);
556556
}
557-
if (peerGroup != null) {
557+
if (peerGroup != null) {
558558
peerGroup.removeConnectedEventListener(peerConnectedEventListener);
559559
peerGroup.removeChainDownloadStartedEventListener(chainDownloadStartedEventListener);
560560
peerGroup.removeHeadersDownloadStartedEventListener(headersDownloadStartedEventListener);
@@ -597,6 +597,7 @@ public void notifyNewBestBlock(StoredBlock block) throws VerificationException {
597597
public final PeerConnectedEventListener peerConnectedEventListener = new PeerConnectedEventListener() {
598598
@Override
599599
public void onPeerConnected(Peer peer, int peerCount) {
600+
if (peerGroup == null) return; // closed during shutdown
600601
downloadPeer = peerGroup.getDownloadPeer();
601602
log.info("peer connected and setting download peer to {} with onPeerConnected", downloadPeer);
602603
}
@@ -605,6 +606,7 @@ public void onPeerConnected(Peer peer, int peerCount) {
605606
final PeerDisconnectedEventListener peerDisconnectedEventListener = new PeerDisconnectedEventListener() {
606607
@Override
607608
public void onPeerDisconnected(Peer peer, int peerCount) {
609+
if (peerGroup == null) return; // closed during shutdown
608610
if (downloadPeer == peer) {
609611
downloadPeer = peerGroup.getDownloadPeer();
610612
log.info("setting download peer to {} with onPeerDisconnected, previously was {}", downloadPeer, peer);
@@ -897,5 +899,7 @@ public void close() {
897899
retryFuture.cancel(true);
898900
retryFuture = null;
899901
}
902+
peerGroup = null;
903+
blockChain = null;
900904
}
901905
}

‎core/src/main/java/org/bitcoinj/evolution/QuorumRotationState.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -445,13 +445,20 @@ public boolean isSynced() {
445445
if (blockChain != null && !params.isDIP0024Active(blockChain.getBestChainHeight()))
446446
return true;
447447

448-
if(mnListAtH.getHeight() == -1)
448+
if (mnListAtH.getHeight() == -1)
449449
return false;
450450

451-
if (peerGroup == null)
451+
if (blockChain == null)
452452
return false;
453453

454-
int mostCommonHeight = peerGroup.getMostCommonHeight();
454+
if (!blockChain.isInitialHeaderSyncComplete()) {
455+
return false;
456+
}
457+
458+
// Use local chain height instead of peerGroup.getMostCommonHeight() to avoid
459+
// acquiring the PeerGroup lock, which can deadlock during shutdown or when the
460+
// PeerGroup thread holds it while blocked on SPVBlockStore I/O.
461+
int mostCommonHeight = blockChain.getBestChainHeight();
455462

456463
// determine when the last QR height was
457464
LLMQParameters llmqParameters = params.getLlmqs().get(llmqType);

‎core/src/main/java/org/bitcoinj/evolution/SimplifiedMasternodeListManager.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,8 +453,9 @@ public void close() {
453453
threadPool.shutdownNow(); // Send interrupt signal but don't wait
454454
}
455455
saveNow(); // Always save, regardless of thread pool state
456+
peerGroup = null;
457+
blockChain = null;
456458
super.close();
457-
458459
}
459460
}
460461

0 commit comments

Comments
 (0)