Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug with reading historical data #4745

Merged

Conversation

chimp1984
Copy link
Contributor

Fix bugs with reading historical data:

  • Use fileName not getFileName() in readHistoricalStoreFromResources
  • use complete handler once reading of all historical data is completed where we
    build the ImmutableMaps and complete the readFromResources method

Fixes #4706 (comment)

- Use fileName not getFileName() in readHistoricalStoreFromResources
- use complete handler once reading of all historical data is completed where we
build the ImmutableMaps and complete the readFromResources method
We used a delegate method in P2PService for calling readPersisted on p2PDataStorage and peerManager.
This was from old times when those classed have not been injected classes. The complete handlers got
called from both p2PDataStorage and peerManager but we counted only P2PService as host, so the
countdown completed before the last host was really completed, leading to a nullpointer in
MainView (not always).

We removed now PersistedDataHost interface from P2PService and use P2PDataStorage and PeerManager to be added to hosts.
Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@ripcurlx ripcurlx merged commit 63cae1c into bisq-network:master Nov 3, 2020
@chimp1984 chimp1984 deleted the fix-bug-with-reading-historical-data branch November 3, 2020 21:11
Copy link
Member

@sqrrm sqrrm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@cbeams
Copy link
Contributor

cbeams commented Nov 4, 2020

ACK (after the fact of merging), as I've tested this locally, and the problem I reported in #4706 (comment) is indeed resolved. There is however another problem still persisting, and I'll comment about that back on the #4706 issue.

@ripcurlx ripcurlx added this to the v1.5.0 milestone Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants