Skip to content

Commit e0a3a5b

Browse files
catbrefcatbref
authored
Merge pull request #101 from catbref/master
Fixes and improvements in sync and shutdown
2 parents 4b5f6e9 + f74e4b2 commit e0a3a5b

File tree

16 files changed

+255
-88
lines changed

16 files changed

+255
-88
lines changed

‎Qora/src/api/QoraResource.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
import org.json.simple.JSONValue;
1212

1313
import controller.Controller;
14+
import gui.ClosingDialog;
15+
import gui.Gui;
1416
import lang.Lang;
1517
import settings.Settings;
1618
import utils.APIUtils;
@@ -31,11 +33,11 @@ public String stop()
3133
if(Controller.getInstance().doesWalletExists() && !Controller.getInstance().isWalletUnlocked()) {
3234
throw ApiErrorFactory.getInstance().createError(ApiErrorFactory.ERROR_WALLET_LOCKED);
3335
}
34-
36+
3537
//STOP
36-
Controller.getInstance().stopAll();
38+
Controller.getInstance().stopAll();
3739
System.exit(0);
38-
40+
3941
//RETURN
4042
return String.valueOf(true);
4143
}

‎Qora/src/controller/Controller.java‎

Lines changed: 140 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import java.util.ArrayList;
1717
import java.util.Arrays;
1818
import java.util.Collection;
19+
import java.util.Collections;
1920
import java.util.Date;
2021
import java.util.Enumeration;
2122
import java.util.LinkedHashMap;
@@ -85,17 +86,18 @@
8586
import utils.ObserverMessage;
8687
import utils.Pair;
8788
import utils.SysTray;
89+
import utils.TransactionTimestampComparator;
8890
import utils.UpdateUtil;
8991
import webserver.WebService;
9092

9193
public class Controller extends Observable {
9294

9395
private static final Logger LOGGER = LogManager.getLogger(Controller.class);
94-
private String version = "0.26.10";
95-
private String buildTime = "2018-06-13 07:57:00 UTC";
96+
private String version = "0.26.11";
97+
private String buildTime = "2018-09-25 12:29:00 UTC";
9698
private long buildTimestamp;
9799

98-
public static final String releaseVersion = "0.26.10";
100+
public static final String releaseVersion = "0.26.11";
99101

100102
// TODO ENUM would be better here
101103
public static final int STATUS_NO_CONNECTIONS = 0;
@@ -408,7 +410,7 @@ public void run() {
408410
}
409411
});
410412

411-
// TIMER TO SEND HEIGHT TO NETWORK EVERY 5 MIN
413+
// TIMER TO SEND HEIGHT TO RANDOM PEER
412414

413415
this.timerPeerHeightUpdate.cancel();
414416
this.timerPeerHeightUpdate = new Timer();
@@ -426,7 +428,7 @@ public void run() {
426428
}
427429
};
428430

429-
this.timerPeerHeightUpdate.schedule(action, 5 * 60 * 1000, 5 * 60 * 1000);
431+
this.timerPeerHeightUpdate.schedule(action, 30 * 1000, 30 * 1000);
430432

431433
// REGISTER DATABASE OBSERVER
432434
this.addObserver(DBSet.getInstance().getTransactionMap());
@@ -579,44 +581,50 @@ public void deleteWalletObserver(Observer o) {
579581
}
580582

581583
private boolean isStopping = false;
584+
private Object stoppingLock = new Object();
582585

583586
public void stopAll() {
584-
// PREVENT MULTIPLE CALLS
585-
if (!this.isStopping) {
586-
this.isStopping = true;
587-
588-
// STOP SENDING OUR HEIGHT TO PEERS
589-
this.timerPeerHeightUpdate.cancel();
590-
591-
// STOP BLOCK PROCESSOR
592-
LOGGER.info(Lang.getInstance().translate("Stopping block processor"));
593-
ClosingDialog.getInstance().updateProgress("Stopping block processor");
594-
this.synchronizer.stop();
595-
596-
// STOP BLOCK GENERATOR
597-
LOGGER.info(Lang.getInstance().translate("Stopping block generator"));
598-
ClosingDialog.getInstance().updateProgress("Stopping block generator");
599-
this.blockGenerator.shutdown();
600-
601-
// STOP MESSAGE PROCESSOR
602-
LOGGER.info(Lang.getInstance().translate("Stopping message processor"));
603-
ClosingDialog.getInstance().updateProgress("Stopping message processor");
604-
this.network.stop();
605-
606-
// CLOSE DATABASE
607-
LOGGER.info(Lang.getInstance().translate("Closing database"));
608-
ClosingDialog.getInstance().updateProgress("Closing database");
609-
DBSet.getInstance().close();
587+
// Prevent multiple calls.
588+
// This method can be called via JVM shutdown hook (e.g. signal), API 'stop' or GUI 'exit', among others.
589+
// In particular, ClosingDialog.getInstance() below can trigger a call to stopAll().
590+
// We want all successive calls to block until the first call has finished.
591+
synchronized (this.stoppingLock) {
592+
if (!this.isStopping) {
593+
this.isStopping = true;
594+
595+
// STOP SENDING OUR HEIGHT TO PEERS
596+
this.timerPeerHeightUpdate.cancel();
597+
598+
// STOP BLOCK PROCESSOR
599+
LOGGER.info(Lang.getInstance().translate("Stopping block processor"));
600+
ClosingDialog.getInstance().updateProgress("Stopping block processor");
601+
this.synchronizer.stop();
602+
603+
// STOP BLOCK GENERATOR
604+
LOGGER.info(Lang.getInstance().translate("Stopping block generator"));
605+
ClosingDialog.getInstance().updateProgress("Stopping block generator");
606+
this.blockGenerator.shutdown();
607+
608+
// STOP MESSAGE PROCESSOR
609+
LOGGER.info(Lang.getInstance().translate("Stopping message processor"));
610+
ClosingDialog.getInstance().updateProgress("Stopping message processor");
611+
this.network.stop();
612+
613+
// CLOSE DATABASE
614+
LOGGER.info(Lang.getInstance().translate("Closing database"));
615+
ClosingDialog.getInstance().updateProgress("Closing database");
616+
DBSet.getInstance().close();
610617

611-
// CLOSE WALLET
612-
LOGGER.info(Lang.getInstance().translate("Closing wallet"));
613-
ClosingDialog.getInstance().updateProgress("Closing wallet");
614-
this.wallet.close();
618+
// CLOSE WALLET
619+
LOGGER.info(Lang.getInstance().translate("Closing wallet"));
620+
ClosingDialog.getInstance().updateProgress("Closing wallet");
621+
this.wallet.close();
615622

616-
ClosingDialog.getInstance().updateProgress("Creating database backup");
617-
createDataCheckpoint();
623+
ClosingDialog.getInstance().updateProgress("Creating database backup");
624+
createDataCheckpoint();
618625

619-
LOGGER.info(Lang.getInstance().translate("Closed."));
626+
LOGGER.info(Lang.getInstance().translate("Closed."));
627+
}
620628
}
621629
}
622630

@@ -649,6 +657,19 @@ public List<Peer> getActivePeers() {
649657
}
650658

651659
public void walletSyncStatusUpdate(int height) {
660+
/*
661+
* Prevent deadlock when a new block arrives from network while we're resyncing wallet.
662+
*
663+
* New block arrival locks Controller and wants Synchronizer,
664+
* but it's possible Synchronizer is locked (e.g. by BlockGenerator) while performing a wallet sync
665+
* and this.setChanged() would want a lock on Controller too, causing a deadlock.
666+
*
667+
* We avoid this by testing for block processing status and exiting early.
668+
*/
669+
670+
if (DBSet.getInstance().getBlockMap().isProcessing())
671+
return;
672+
652673
this.setChanged();
653674
this.notifyObservers(new ObserverMessage(ObserverMessage.WALLET_SYNC_STATUS, height));
654675
}
@@ -671,9 +692,6 @@ public void onConnect(Peer peer) {
671692
if (DBSet.getInstance().isStopped())
672693
return;
673694

674-
// GET HEIGHT
675-
int height = this.blockChain.getHeight();
676-
677695
if (NTP.getTime() >= Transaction.getPOWFIX_RELEASE()) {
678696
// SEND FOUNDMYSELF MESSAGE
679697
peer.sendMessage(MessageFactory.getInstance().createFindMyselfMessage(Controller.getInstance().getFoundMyselfID()));
@@ -683,8 +701,19 @@ public void onConnect(Peer peer) {
683701
}
684702

685703
// SEND HEIGHT MESSAGE
686-
LOGGER.trace("Sending our height " + height + " to peer " + peer.getAddress());
687-
peer.sendMessage(MessageFactory.getInstance().createHeightMessage(height));
704+
getSendMyHeightToPeer(peer);
705+
706+
// Resend any unconfirmed transactions
707+
List<Transaction> transactions = DBSet.getInstance().getTransactionMap().getTransactions();
708+
709+
// Sort transactions chronologically
710+
Collections.sort(transactions, new TransactionTimestampComparator());
711+
712+
// Send unconfirmed transactions
713+
for (Transaction transaction : transactions) {
714+
Message message = MessageFactory.getInstance().createTransactionMessage(transaction);
715+
peer.sendMessage(message);
716+
}
688717

689718
if (this.status == STATUS_NO_CONNECTIONS) {
690719
// UPDATE STATUS
@@ -743,12 +772,17 @@ public void onDisconnect(Peer peer) {
743772
// in case they attempt to access DB
744773
if (this.isStopping)
745774
return;
746-
747-
// NOTIFY
748-
this.setChanged();
749-
this.notifyObservers(new ObserverMessage(ObserverMessage.NETWORK_STATUS, this.status));
750775
}
751776
}
777+
778+
// NOTIFY, but in separate thread to avoid MapDB interrupt issue
779+
new Thread() {
780+
@Override
781+
public void run() {
782+
Controller.getInstance().setChanged();
783+
Controller.getInstance().notifyObservers(new ObserverMessage(ObserverMessage.NETWORK_STATUS, Controller.getInstance().status));
784+
}
785+
}.start();
752786
}
753787

754788
public void onError(Peer peer) {
@@ -830,10 +864,25 @@ public void onMessage(Message message) {
830864

831865
case Message.BLOCK_TYPE:
832866

867+
// Don't process if we're synchronizing
868+
if (this.status == STATUS_SYNCHRONIZING)
869+
break;
870+
833871
BlockMessage blockMessage = (BlockMessage) message;
834872

835-
// ASK BLOCK FROM BLOCKCHAIN
873+
// Get block from message
836874
block = blockMessage.getBlock();
875+
LOGGER.trace("Received block from peer " + message.getSender().getAddress());
876+
877+
// Compare to our blockchain tip
878+
Block blockchainTip = this.blockChain.getLastBlock();
879+
if (blockchainTip.getHeight() == blockMessage.getHeight() && Arrays.equals(blockchainTip.getSignature(), block.getSignature())) {
880+
// We have this block already but update our peer DB to reflect peer's height anyway
881+
synchronized (this.peerHeight) {
882+
this.peerHeight.put(message.getSender(), blockMessage.getHeight());
883+
}
884+
break;
885+
}
837886

838887
boolean isNewBlockValid = this.blockChain.isNewBlockValid(block);
839888

@@ -846,6 +895,18 @@ public void onMessage(Message message) {
846895
if (this.isProcessingWalletSynchronize())
847896
break;
848897

898+
/*
899+
* Prevent deadlock when a new block arrives from network while we're resyncing wallet.
900+
*
901+
* New block arrival locks Controller and wants Synchronizer,
902+
* but it's possible Synchronizer is locked (e.g. by BlockGenerator) while performing a wallet sync
903+
* and this.setChanged() would want a lock on Controller too, causing a deadlock.
904+
*
905+
* We avoid this by testing for block processing status and exiting early.
906+
*/
907+
if (DBSet.getInstance().getBlockMap().isProcessing())
908+
break;
909+
849910
// CHECK IF VALID
850911
if (isNewBlockValid && this.synchronizer.process(block)) {
851912
LOGGER.info(Lang.getInstance().translate("received new valid block") + " " + block.getHeight());
@@ -858,6 +919,9 @@ public void onMessage(Message message) {
858919
excludes.add(message.getSender());
859920
this.network.broadcast(message, excludes);
860921

922+
// Let sender know we've updated
923+
this.getSendMyHeightToPeer(message.getSender());
924+
861925
// UPDATE ALL PEER HEIGHTS TO OUR HEIGHT
862926
/*
863927
* synchronized(this.peerHeight) { for(Peer peer: this.peerHeight.keySet()) { this.peerHeight.put(peer, this.blockChain.getHeight()); }
@@ -983,9 +1047,22 @@ public void update() {
9831047
peer = this.getMaxHeightPeer();
9841048

9851049
if (peer != null) {
1050+
// Make a note of pre-sync height so we can tell if anything happened
1051+
int preSyncHeight = this.blockChain.getHeight();
1052+
9861053
// SYNCHRONIZE FROM PEER
9871054
LOGGER.info("Synchronizing using peer " + peer.getAddress().getHostAddress() + " with height " + peerHeight.get(peer) + " - ping " + peer.getPing() + "ms");
9881055
this.synchronizer.synchronize(peer);
1056+
1057+
// If our height has changed, notify our peers
1058+
if (this.blockChain.getHeight() > preSyncHeight) {
1059+
Block blockchainTip = this.blockChain.getLastBlock();
1060+
LOGGER.debug("Sending our new height " + blockchainTip.getHeight() + " to peers");
1061+
Message message = MessageFactory.getInstance().createHeightMessage(blockchainTip.getHeight());
1062+
1063+
List<Peer> excludes = new ArrayList<Peer>();
1064+
this.network.broadcast(message, excludes);
1065+
}
9891066
}
9901067

9911068
Thread.sleep(5 * 1000);
@@ -999,6 +1076,8 @@ public void update() {
9991076
// DISHONEST PEER
10001077
this.network.onError(peer, e.getMessage());
10011078
}
1079+
1080+
return;
10021081
}
10031082

10041083
if (this.peerHeight.size() == 0) {
@@ -1354,10 +1433,17 @@ public long getNextBlockGeneratingBalance(Block parent) {
13541433
// FORGE
13551434

13561435
public void newBlockGenerated(Block newBlock) {
1357-
this.synchronizer.process(newBlock);
1436+
// Only process if we have connections and are not synchronizing
1437+
if (this.status == STATUS_OK) {
1438+
if (this.synchronizer.process(newBlock)) {
1439+
LOGGER.info("Forged new block " + newBlock.getHeight());
13581440

1359-
// BROADCAST
1360-
this.broadcastBlock(newBlock);
1441+
// BROADCAST
1442+
this.broadcastBlock(newBlock);
1443+
} else {
1444+
LOGGER.info("Couldn't forge new block");
1445+
}
1446+
}
13611447
}
13621448

13631449
public List<Transaction> getUnconfirmedTransactions() {
@@ -1655,6 +1741,9 @@ public Pair<Transaction, Integer> sendMessage(PrivateKeyAccount sender, Account
16551741

16561742
public Block getBlockByHeight(int parseInt) {
16571743
byte[] b = DBSet.getInstance().getHeightMap().getBlockByHeight(parseInt);
1744+
if (b == null)
1745+
return null;
1746+
16581747
return DBSet.getInstance().getBlockMap().get(b);
16591748
}
16601749

‎Qora/src/database/DBListValueMap.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ protected L getListForAdd(K key) {
7878
L newList = this.newListValue();
7979

8080
// If we previous marked entry as deleted then we're essentially recreating a new list
81-
if (this.deleted != null && this.deleted.contains(key))
81+
if (this.deletedContains(key))
8282
return newList;
8383

8484
// If the parent map contains a list for key then we need to duplicate its entries
@@ -148,7 +148,7 @@ protected L getListForRemove(K key) {
148148
return this.map.get(key);
149149

150150
// If we previously marked entry as deleted then we have no list
151-
if (this.deleted != null && this.deleted.contains(key))
151+
if (this.deletedContains(key))
152152
return null;
153153

154154
// If we have no parent or parent has no entry then there's no list

0 commit comments

Comments
 (0)