From 0520942a4c12bdb23014a4f4e965c81e99e6a225 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Mon, 7 Dec 2020 11:42:19 -0500 Subject: [PATCH 1/2] Prevent that we write data at shutdown before we have read the data. Fixes https://github.com/bisq-network/bisq/issues/4844 --- .../bisq/common/persistence/PersistenceManager.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/common/src/main/java/bisq/common/persistence/PersistenceManager.java b/common/src/main/java/bisq/common/persistence/PersistenceManager.java index 214b6b8768..b052c26ff8 100644 --- a/common/src/main/java/bisq/common/persistence/PersistenceManager.java +++ b/common/src/main/java/bisq/common/persistence/PersistenceManager.java @@ -110,7 +110,11 @@ public class PersistenceManager { // For Priority.HIGH data we want to write to disk in any case to be on the safe side if we might have missed // a requestPersistence call after an important state update. Those are usually rather small data stores. // Otherwise we only persist if requestPersistence was called since the last persist call. - if (persistenceManager.source.flushAtShutDown || persistenceManager.persistenceRequested) { + // We also check if we have called read already to avoid a very early write attempt before we have ever + // read the data, which would lead to a write of empty data + // (fixes https://github.com/bisq-network/bisq/issues/4844). + if (persistenceManager.readCalled.get() && + (persistenceManager.source.flushAtShutDown || persistenceManager.persistenceRequested)) { // We always get our completeHandler called even if exceptions happen. In case a file write fails // we still call our shutdown and count down routine as the completeHandler is triggered in any case. @@ -184,6 +188,7 @@ public class PersistenceManager { private Timer timer; private ExecutorService writeToDiskExecutor; public final AtomicBoolean initCalled = new AtomicBoolean(false); + public final AtomicBoolean readCalled = new AtomicBoolean(false); /////////////////////////////////////////////////////////////////////////////////////////// @@ -293,7 +298,9 @@ public class PersistenceManager { // Currently used by tests and monitor. Should be converted to the threaded API as well. @Nullable public T getPersisted() { - return getPersisted(checkNotNull(fileName)); + T persisted = getPersisted(checkNotNull(fileName)); + readCalled.set(true); + return persisted; } @Nullable From 15ed165cddb04a8cb6253b52722847c9d10c2b60 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Mon, 7 Dec 2020 12:05:28 -0500 Subject: [PATCH 2/2] Move readCalled.set(true); to getPersisted(String fileName) method. Note: It must be before the null check as no existing file must not prevent write. --- .../java/bisq/common/persistence/PersistenceManager.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/common/src/main/java/bisq/common/persistence/PersistenceManager.java b/common/src/main/java/bisq/common/persistence/PersistenceManager.java index b052c26ff8..8300e9c336 100644 --- a/common/src/main/java/bisq/common/persistence/PersistenceManager.java +++ b/common/src/main/java/bisq/common/persistence/PersistenceManager.java @@ -298,9 +298,7 @@ public class PersistenceManager { // Currently used by tests and monitor. Should be converted to the threaded API as well. @Nullable public T getPersisted() { - T persisted = getPersisted(checkNotNull(fileName)); - readCalled.set(true); - return persisted; + return getPersisted(checkNotNull(fileName)); } @Nullable @@ -310,6 +308,8 @@ public class PersistenceManager { return null; } + readCalled.set(true); + File storageFile = new File(dir, fileName); if (!storageFile.exists()) { return null;