Skip to content

Commit

Permalink
Fix Multivalue Slice Deadlock (#13087)
Browse files Browse the repository at this point in the history
* fix deadlock

* gofmt

* lint

(cherry picked from commit f91efaf)
  • Loading branch information
nisdas authored and prestonvanloon committed Oct 23, 2023
1 parent 747d7b5 commit bc9151d
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 7 deletions.
38 changes: 38 additions & 0 deletions beacon-chain/state/state-native/references_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/prysmaticlabs/go-bitfield"
"github.com/prysmaticlabs/prysm/v4/beacon-chain/state"
"github.com/prysmaticlabs/prysm/v4/beacon-chain/state/state-native/types"
"github.com/prysmaticlabs/prysm/v4/config/features"
"github.com/prysmaticlabs/prysm/v4/encoding/bytesutil"
ethpb "github.com/prysmaticlabs/prysm/v4/proto/prysm/v1alpha1"
"github.com/prysmaticlabs/prysm/v4/testing/assert"
Expand Down Expand Up @@ -1015,6 +1016,43 @@ func TestValidatorReferences_RemainsConsistent_Bellatrix(t *testing.T) {
}))
}

func TestValidatorReferences_ApplyValidator_BalancesRead(t *testing.T) {
resetCfg := features.InitWithReset(&features.Flags{
EnableExperimentalState: true,
})
defer resetCfg()
s, err := InitializeFromProtoUnsafeAltair(&ethpb.BeaconStateAltair{
Validators: []*ethpb.Validator{
{PublicKey: []byte{'A'}},
{PublicKey: []byte{'B'}},
{PublicKey: []byte{'C'}},
{PublicKey: []byte{'D'}},
{PublicKey: []byte{'E'}},
},
Balances: []uint64{0, 0, 0, 0, 0},
})
require.NoError(t, err)
a, ok := s.(*BeaconState)
require.Equal(t, true, ok)

// Create a second state.
copied := a.Copy()
b, ok := copied.(*BeaconState)
require.Equal(t, true, ok)

// Modify all validators from copied state, it should not deadlock.
assert.NoError(t, b.ApplyToEveryValidator(func(idx int, val *ethpb.Validator) (bool, *ethpb.Validator, error) {
b, err := b.BalanceAtIndex(0)
if err != nil {
return false, nil, err
}
newVal := ethpb.CopyValidator(val)
newVal.EffectiveBalance += b
val.EffectiveBalance += b
return true, val, nil
}))
}

// assertRefCount checks whether reference count for a given state
// at a given index is equal to expected amount.
func assertRefCount(t *testing.T, b *BeaconState, idx types.FieldIndex, want uint) {
Expand Down
7 changes: 0 additions & 7 deletions beacon-chain/state/state-native/setters_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,30 +40,23 @@ func (b *BeaconState) SetValidators(val []*ethpb.Validator) error {
func (b *BeaconState) ApplyToEveryValidator(f func(idx int, val *ethpb.Validator) (bool, *ethpb.Validator, error)) error {
var changedVals []uint64
if features.Get().EnableExperimentalState {
b.lock.Lock()

l := b.validatorsMultiValue.Len(b)
for i := 0; i < l; i++ {
v, err := b.validatorsMultiValue.At(b, uint64(i))
if err != nil {
b.lock.Unlock()
return err
}
changed, newVal, err := f(i, v)
if err != nil {
b.lock.Unlock()
return err
}
if changed {
changedVals = append(changedVals, uint64(i))
if err = b.validatorsMultiValue.UpdateAt(b, uint64(i), newVal); err != nil {
b.lock.Unlock()
return errors.Wrapf(err, "could not update validator at index %d", i)
}
}
}

b.lock.Unlock()
} else {
b.lock.Lock()

Expand Down

0 comments on commit bc9151d

Please sign in to comment.