Skip to content

[Bug] Fix wallet data race #2923

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.

Sign up for GitHub

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

Jump to bottom
Merged
Fuzzbawls merged 2 commits into from
Apr 20, 2024
Merged

[Bug] Fix wallet data race #2923

Fuzzbawls merged 2 commits into from
Apr 20, 2024

Conversation

@panleone
Copy link

@panleone panleone commented Apr 5, 2024

First commit is a small refactor: the global variable nTimeBestReceived is now a private member of the CWallet class, as it was used only there.

Second commit: fix the following data race by making nTimeBestReceived atomic

WARNING: ThreadSanitizer: data race (pid=18270)
  Write of size 8 at 0x7b6800000ed8 by thread T16:
    #0 CWallet::UpdatedBlockTip(CBlockIndex const*, CBlockIndex const*, bool) wallet/wallet.cpp:2094 (pivxd+0x5058c9)
    #1 void std::__invoke_impl<void, void (CValidationInterface::*&)(CBlockIndex const*, CBlockIndex const*, bool), CValidationInterface*&, CBlockIndex const*, CBlockIndex const*, bool>(std::__invoke_memfun_deref, void (CValidationInterface::*&)(CBlockIndex const*, CBlockIndex const*, bool), CValidationInterface*&, CBlockIndex const*&&, CBlockIndex const*&&, bool&&) /usr/include/c++/12/bits/invoke.h:74 (pivxd+0x2ee1c0)
    #2 std::__invoke_result<void (CValidationInterface::*&)(CBlockIndex const*, CBlockIndex const*, bool), CValidationInterface*&, CBlockIndex const*, CBlockIndex const*, bool>::type std::__invoke<void (CValidationInterface::*&)(CBlockIndex const*, CBlockIndex const*, bool), CValidationInterface*&, CBlockIndex const*, CBlockIndex const*, bool>(void (CValidationInterface::*&)(CBlockIndex const*, CBlockIndex const*, bool), CValidationInterface*&, CBlockIndex const*&&, CBlockIndex const*&&, bool&&) /usr/include/c++/12/bits/invoke.h:96 (pivxd+0x2ee25a)
    #3 void std::_Bind<void (CValidationInterface::*(CValidationInterface*, std::_Placeholder<1>, std::_Placeholder<2>, std::_Placeholder<3>))(CBlockIndex const*, CBlockIndex const*, bool)>::__call<void, CBlockIndex const*&&, CBlockIndex const*&&, bool&&, 0ul, 1ul, 2ul, 3ul>(std::tuple<CBlockIndex const*&&, CBlockIndex const*&&, bool&&>&&, std::_Index_tuple<0ul, 1ul, 2ul, 3ul>) /usr/include/c++/12/functional:484 (pivxd+0x2ee25a)
    ...

  Previous read of size 8 at 0x7b6800000ed8 by thread T35 (mutexes: write M134298, write M132880):
    #0 CWallet::ResendWalletTransactions(CConnman*) wallet/wallet.cpp:2065 (pivxd+0x515b25)
    #1 void std::__invoke_impl<void, void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*>(std::__invoke_memfun_deref, void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*&&) /usr/include/c++/12/bits/invoke.h:74 (pivxd+0x2eef45)
    #2 std::__invoke_result<void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*>::type std::__invoke<void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*>(void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*&&) /usr/include/c++/12/bits/invoke.h:96 (pivxd+0x2eefb9)
    #3 void std::_Bind<void (CValidationInterface::*(CValidationInterface*, std::_Placeholder<1>))(CConnman*)>::__call<void, CConnman*&&, 0ul, 1ul>(std::tuple<CConnman*&&>&&, std::_Index_tuple<0ul, 1ul>) /usr/include/c++/12/functional:484 (pivxd+0x2eefb9)
    ...

@panleone panleone added the Bug label Apr 5, 2024
@panleone panleone added this to the 6.0.0 milestone Apr 5, 2024
@panleone panleone self-assigned this Apr 5, 2024
Duddino
Duddino approved these changes Apr 9, 2024
View reviewed changes
Copy link
Member

@Duddino Duddino left a comment

Choose a reason for hiding this comment

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

utACK f9a6ebd

Liquid369
Liquid369 approved these changes Apr 10, 2024
View reviewed changes
Copy link
Member

@Liquid369 Liquid369 left a comment

Choose a reason for hiding this comment

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

tACK f9a6ebd

Fuzzbawls
Fuzzbawls approved these changes Apr 20, 2024
View reviewed changes
Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

ACK f9a6ebd

@Fuzzbawls Fuzzbawls merged commit 694eb59 into PIVX-Project:master Apr 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Fuzzbawls Fuzzbawls Fuzzbawls approved these changes

@Liquid369 Liquid369 Liquid369 approved these changes

@Duddino Duddino Duddino approved these changes

Assignees

@panleone panleone

Labels

Bug

Projects

None yet

Milestone

6.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@panleone @Fuzzbawls @Liquid369 @Duddino