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

[test] fix miner_tests block generation race condition. #2233

Merged
merged 1 commit into from
Mar 10, 2021

Conversation

furszy
Copy link

@furszy furszy commented Mar 7, 2021

Took a while to understand the UB.. but this should fix it. Based on the results thrown by #2220.

As we are "faking" the block creation based on the same CBlock shared_ptr and the pointer is shared across multiple threads, there is a possible race condition with the scheduler thread when we clear the transactions vector to add the new coinbase transaction for the next "new" block. If the secondary thread is not fast enough, as we are doing all in the same loop non-stop, the scheduler thread could try to access to the block ptr vtx a millisecond later when there is no transactions, causing the UB due the vector brackets operator access on an empty container.

@furszy furszy self-assigned this Mar 7, 2021
As we are "faking" the block creation based on the same CBlock shared_ptr and the pointer is shared across multiple threads, there is a possible race condition with the scheduler thread when we clear the transactions vector to add the new coinbase transaction for the next "new" block. If secondary thread is not fast enough, as we are doing all in the same loop non-stop, the thread could try to access to the block ptr vtx where there is no transactions, causing an UB.
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.

utACK f12bd4c

@random-zebra random-zebra added this to the 5.1.0 milestone Mar 10, 2021
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

utACK f12bd4c and merging...

@random-zebra random-zebra merged commit e9461b3 into PIVX-Project:master Mar 10, 2021
@furszy furszy deleted the 2020_fix_miner_test_UB branch November 29, 2022 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants