Skip to content

Bugfix: race condition of two threads using same RandomX VM#1021

Merged
seanPhill merged 1 commit intoVeil-Project:masterfrom
us77ipis:fix_randomx_vm_race_condition
Feb 2, 2023
Merged

Bugfix: race condition of two threads using same RandomX VM#1021
seanPhill merged 1 commit intoVeil-Project:masterfrom
us77ipis:fix_randomx_vm_race_condition

Conversation

@us77ipis
Copy link
Contributor

@us77ipis us77ipis commented Nov 4, 2022

Race condition between

randomx_calculate_hash(GetMyMachineValidating(), &hash_blob, sizeof uint256(), hash);
and
randomx_calculate_hash(GetMyMachineValidating(), &hash_blob, sizeof uint256(), hash);

which use the same RandomX VM but the first one correctly acquires the lock cs_randomx_validator before, while the second doesn't acquire the lock.

This bug caused the daemon / wallet to randomly crash.

Copy link
Collaborator

@Zannick Zannick left a comment

Choose a reason for hiding this comment

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

utACK 140b2fe

@seanPhill seanPhill added the QA: Pending QA is waiting a response/confirmation from developers label Dec 21, 2022
@seanPhill seanPhill added QA: In Progress This is being tested by the QA team. KEEP CALM and RELAX. and removed QA: Pending QA is waiting a response/confirmation from developers labels Jan 31, 2023
@seanPhill
Copy link
Collaborator

I have been running this for some weeks, but haven't done a lot of RandomX mining.
I am stepping up this mining on testnet.

@seanPhill seanPhill added QA: Passed This has passed QA testing and can be merged to master and removed QA: In Progress This is being tested by the QA team. KEEP CALM and RELAX. labels Feb 2, 2023
@seanPhill
Copy link
Collaborator

I ran this on testnet and mined a lot of RandomX without any problems. Debug log entries look fine.

@seanPhill seanPhill merged commit df7b820 into Veil-Project:master Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

QA: Passed This has passed QA testing and can be merged to master

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants