[bisq-network/bisq] (6/6) Persist changes to ProtectedStoragePayload objects implementing PersistablePayload (#3640)

Florian Reimair notifications at github.com
Mon Nov 25 11:26:24 UTC 2019


freimair requested changes on this pull request.



> @@ -418,9 +418,8 @@ public boolean addProtectedStorageEntry(ProtectedStorageEntry protectedStorageEn
 
         // Persist ProtectedStorageEntrys carrying PersistablePayload payloads and signal listeners on changes
         if (protectedStoragePayload instanceof PersistablePayload) {
-            ProtectedStorageEntry previous = protectedDataStoreService.putIfAbsent(hashOfPayload, protectedStorageEntry);
-            if (previous == null)
-                protectedDataStoreListeners.forEach(e -> e.onAdded(protectedStorageEntry));
+            protectedDataStoreService.put(hashOfPayload, protectedStorageEntry);

This means that we will have a lot more save procedures. Looking at the code in `FileManager:75` ff. I feel not confident to do this change now as we significantly up the risk of having corrupted data stores - which is very bad.

Just by a quick look at the filemanager code, I doubt that this is thread-safe. There seems to be an obvious race condition in 79-83. Can we have that tested and fixed if needed before we apply this patch?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/3640#pullrequestreview-322206990
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20191125/29f77631/attachment-0001.html>


More information about the bisq-github mailing list