Skip to content

fix: missing lock cs_wallet in rpc/evo - #6002

Merged
PastaPastaPasta merged 1 commit into
dashpay:developfrom
knst:fix-locks-rpc-evo
May 3, 2024
Merged

fix: missing lock cs_wallet in rpc/evo#6002
PastaPastaPasta merged 1 commit into
dashpay:developfrom
knst:fix-locks-rpc-evo

Conversation

@knst

@knst knst commented Apr 30, 2024

Copy link
Copy Markdown
Collaborator

Issue being fixed or feature implemented

Changing order of locking cs_main and cs_wallet to prevent a deadlock.

There's call protx_list -> BuildDMNListEntry -> CheckWalletOwnsScript:

return WITH_LOCK(pwallet->cs_wallet, return pwallet->IsMine(script)) == isminetype::ISMINE_SPENDABLE; 

It can cause a deadlock due to wrong order of locks (cs_wallet supposed to be blocked in prior of cs_main)

What was done?

This PR adds and extra lock of cs_wallet and reduce scope of cs_main for a bit

How Has This Been Tested?

Deadlock warning is reproduced with this PR: #6003

Breaking Changes

N/A

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone

@knst knst added this to the 21 milestone Apr 30, 2024
@knst
knst force-pushed the fix-locks-rpc-evo branch from 3c016c0 to 1aa9087 Compare April 30, 2024 13:38
@knst
knst marked this pull request as draft April 30, 2024 14:08
It also reduce scope of both locks
@knst
knst force-pushed the fix-locks-rpc-evo branch from 7d84ab9 to 10869ff Compare May 2, 2024 16:49
@knst
knst marked this pull request as ready for review May 2, 2024 18:32
@knst
knst requested review from PastaPastaPasta and UdjinM6 and removed request for UdjinM6 May 2, 2024 18:32
Comment thread src/rpc/evo.cpp
@knst
knst requested a review from UdjinM6 May 3, 2024 10:25

@UdjinM6 UdjinM6 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

utACK

@PastaPastaPasta
PastaPastaPasta merged commit 3b2c8bd into dashpay:develop May 3, 2024
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.

3 participants