Skip to content

Commit

Permalink
Merge pull request #2460 from onflow/supun/resource-reference-invalid…
Browse files Browse the repository at this point in the history 
…ation

Invalidate references to nested resources upon the move of the outer resource
  • Loading branch information
@SupunS
SupunS authored Aug 9, 2023
2 parents 07f5125 + e90ddca commit 341b7b5
Show file tree
Hide file tree
Showing 9 changed files with 413 additions and 135 deletions.
4 changes: 2 additions & 2 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ require (
github.com/go-test/deep v1.0.5
github.com/google/go-cmp v0.5.9 // indirect
github.com/leanovate/gopter v0.2.9
github.com/onflow/atree v0.5.0
github.com/onflow/atree v0.6.1-0.20230711151834-86040b30171f
github.com/rivo/uniseg v0.2.1-0.20211004051800-57c86be7915a
github.com/schollz/progressbar/v3 v3.8.3
github.com/stretchr/testify v1.8.1
github.com/stretchr/testify v1.8.4
github.com/tidwall/pretty v1.2.1
github.com/turbolent/prettier v0.0.0-20220320183459-661cc755135d
go.opentelemetry.io/otel v1.8.0
Expand Down
13 changes: 4 additions & 9 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ github.com/mattn/go-tty v0.0.3/go.mod h1:ihxohKRERHTVzN+aSVRwACLCeqIoZAWpoICkkvr
github.com/mitchellh/colorstring v0.0.0-20190213212951-d06e56a500db h1:62I3jR2EmQ4l5rM/4FEfDWcRD+abF5XlKShorW5LRoQ=
github.com/mitchellh/colorstring v0.0.0-20190213212951-d06e56a500db/go.mod h1:l0dey0ia/Uv7NcFFVbCLtqEBQbrT4OCwCSKTEv6enCw=
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWbfPhv4DMiApHyliiK5xCTNVSPiaAs=
github.com/onflow/atree v0.5.0 h1:y3lh8hY2fUo8KVE2ALVcz0EiNTq0tXJ6YTXKYVDA+3E=
github.com/onflow/atree v0.5.0/go.mod h1:gBHU0M05qCbv9NN0kijLWMgC47gHVNBIp4KmsVFi0tc=
github.com/onflow/atree v0.6.1-0.20230711151834-86040b30171f h1:Z8/PgTqOgOg02MTRpTBYO2k16FE6z4wEOtaC2WBR9Xo=
github.com/onflow/atree v0.6.1-0.20230711151834-86040b30171f/go.mod h1:xvP61FoOs95K7IYdIYRnNcYQGf4nbF/uuJ0tHf4DRuM=
github.com/pkg/term v1.1.0 h1:xIAAdCMh3QIAy+5FrE8Ad8XoDhEU4ufwbaSozViP9kk=
github.com/pkg/term v1.1.0/go.mod h1:E25nymQcrSllhX42Ok8MRm1+hyBdHY0dCeiKZ9jpNGw=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
Expand All @@ -70,15 +70,11 @@ github.com/schollz/progressbar/v3 v3.8.3 h1:FnLGl3ewlDUP+YdSwveXBaXs053Mem/du+wr
github.com/schollz/progressbar/v3 v3.8.3/go.mod h1:pWnVCjSBZsT2X3nx9HfRdnCDrpbevliMeoEVhStwHko=
github.com/sergi/go-diff v1.2.0 h1:XU+rvMAioB0UC3q1MFrIQy4Vo5/4VsRDQQXHsEya6xQ=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c=
github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk=
github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
github.com/texttheater/golang-levenshtein/levenshtein v0.0.0-20200805054039-cae8b0eaed6c h1:HelZ2kAFadG0La9d+4htN4HzQ68Bm2iM9qKMSMES6xg=
github.com/texttheater/golang-levenshtein/levenshtein v0.0.0-20200805054039-cae8b0eaed6c/go.mod h1:JlzghshsemAMDGZLytTFY8C1JQxQPhnatWqNwUXjggo=
github.com/tidwall/pretty v1.2.1 h1:qjsOFOWWQl+N3RsoF5/ssm1pHmJJwhjlSbZ51I6wMl4=
Expand Down Expand Up @@ -157,7 +153,6 @@ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b h1:QRR6H1YWRnHb4Y/HeNFCTJLFVxaq6wH4YuVdsUOr75U=
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
lukechampine.com/blake3 v1.1.7 h1:GgRMhmdsuK8+ii6UZFDL8Nb+VyMwadAgcJyfYHxG6n0=
4 changes: 4 additions & 0 deletions runtime/cmd/decode-state-values/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,10 @@ func (s *slabStorage) Count() int {
return len(storage)
}

func (s *slabStorage) RetrieveIfLoaded(id atree.StorageID) atree.Slab {
panic("unexpected RetrieveIfLoaded call")
}

// interpreterStorage

type interpreterStorage struct {
Expand Down
67 changes: 54 additions & 13 deletions runtime/interpreter/interpreter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5048,25 +5048,66 @@ func (interpreter *Interpreter) trackReferencedResourceKindedValue(
values[value] = struct{}{}
}

func (interpreter *Interpreter) updateReferencedResource(
currentStorageID atree.StorageID,
newStorageID atree.StorageID,
updateFunc func(value ReferenceTrackedResourceKindedValue),
) {
values := interpreter.SharedState.referencedResourceKindedValues[currentStorageID]
func (interpreter *Interpreter) invalidateReferencedResources(value Value, destroyed bool) {
// skip non-resource typed values
if !value.IsResourceKinded(interpreter) {
return
}

var storageID atree.StorageID

switch value := value.(type) {
case *CompositeValue:
value.ForEachLoadedField(interpreter, func(_ string, fieldValue Value) {
interpreter.invalidateReferencedResources(fieldValue, destroyed)
})
storageID = value.StorageID()
case *DictionaryValue:
value.IterateLoaded(interpreter, func(_, value Value) (resume bool) {
interpreter.invalidateReferencedResources(value, destroyed)
return true
})
storageID = value.StorageID()
case *ArrayValue:
value.IterateLoaded(interpreter, func(element Value) (resume bool) {
interpreter.invalidateReferencedResources(element, destroyed)
return true
})
storageID = value.StorageID()
case *SomeValue:
interpreter.invalidateReferencedResources(value.value, destroyed)
return
default:
// skip non-container typed values.
return
}

values := interpreter.SharedState.referencedResourceKindedValues[storageID]
if values == nil {
return
}

for value := range values { //nolint:maprange
updateFunc(value)
switch value := value.(type) {
case *CompositeValue:
value.dictionary = nil
value.isDestroyed = destroyed
case *DictionaryValue:
value.dictionary = nil
value.isDestroyed = destroyed
case *ArrayValue:
value.array = nil
value.isDestroyed = destroyed
default:
panic(errors.NewUnreachableError())
}
}

// If the move is to a new location, then the resources are already cleared via the update function above.
// So no need to track those stale resources anymore.
if newStorageID != currentStorageID {
interpreter.SharedState.referencedResourceKindedValues[newStorageID] = values
interpreter.SharedState.referencedResourceKindedValues[currentStorageID] = nil
}
// The old resource instances are already cleared/invalidated above.
// So no need to track those stale resources anymore. We will not need to update/clear them again.
// Therefore, remove them from the mapping.
// This is only to allow GC. No impact to the behavior.
delete(interpreter.SharedState.referencedResourceKindedValues, storageID)
}

// startResourceTracking starts tracking the life-span of a resource.
Expand Down
153 changes: 50 additions & 103 deletions runtime/interpreter/value.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1720,8 +1720,20 @@ func (v *ArrayValue) Accept(interpreter *Interpreter, visitor Visitor) {
}

func (v *ArrayValue) Iterate(interpreter *Interpreter, f func(element Value) (resume bool)) {
v.iterate(interpreter, v.array.Iterate, f)
}

func (v *ArrayValue) IterateLoaded(interpreter *Interpreter, f func(element Value) (resume bool)) {
v.iterate(interpreter, v.array.IterateLoadedValues, f)
}

func (v *ArrayValue) iterate(
interpreter *Interpreter,
atreeIterate func(fn atree.ArrayIterationFunc) error,
f func(element Value) (resume bool),
) {
iterate := func() {
err := v.array.Iterate(func(element atree.Value) (resume bool, err error) {
err := atreeIterate(func(element atree.Value) (resume bool, err error) {
// atree.Array iteration provides low-level atree.Value,
// convert to high-level interpreter.Value

Expand Down Expand Up @@ -1820,26 +1832,11 @@ func (v *ArrayValue) Destroy(interpreter *Interpreter, locationRange LocationRan

v.isDestroyed = true

interpreter.invalidateReferencedResources(v, true)

if config.InvalidatedResourceValidationEnabled {
v.array = nil
}

interpreter.updateReferencedResource(
storageID,
storageID,
func(value ReferenceTrackedResourceKindedValue) {
arrayValue, ok := value.(*ArrayValue)
if !ok {
panic(errors.NewUnreachableError())
}

arrayValue.isDestroyed = true

if config.InvalidatedResourceValidationEnabled {
arrayValue.array = nil
}
},
)
}

func (v *ArrayValue) IsDestroyed() bool {
Expand Down Expand Up @@ -2706,28 +2703,14 @@ func (v *ArrayValue) Transfer(
// This allows raising an error when the resource array is attempted
// to be transferred/moved again (see beginning of this function)

interpreter.invalidateReferencedResources(v, false)

if config.InvalidatedResourceValidationEnabled {
v.array = nil
} else {
v.array = array
res = v
}

newStorageID := array.StorageID()

interpreter.updateReferencedResource(
currentStorageID,
newStorageID,
func(value ReferenceTrackedResourceKindedValue) {
arrayValue, ok := value.(*ArrayValue)
if !ok {
panic(errors.NewUnreachableError())
}

// Any kind of move would invalidate the references.
arrayValue.array = nil
},
)
}

if res == nil {
Expand Down Expand Up @@ -16307,27 +16290,11 @@ func (v *CompositeValue) Destroy(interpreter *Interpreter, locationRange Locatio

v.isDestroyed = true

interpreter.invalidateReferencedResources(v, true)

if config.InvalidatedResourceValidationEnabled {
v.dictionary = nil
}

interpreter.updateReferencedResource(
storageID,
storageID,
func(value ReferenceTrackedResourceKindedValue) {
compositeValue, ok := value.(*CompositeValue)
if !ok {
panic(errors.NewUnreachableError())
}

compositeValue.isDestroyed = true

if config.InvalidatedResourceValidationEnabled {
compositeValue.dictionary = nil
}
},
)

}

func (v *CompositeValue) getBuiltinMember(interpreter *Interpreter, locationRange LocationRange, name string) Value {
Expand Down Expand Up @@ -17088,28 +17055,14 @@ func (v *CompositeValue) Transfer(
// This allows raising an error when the resource is attempted
// to be transferred/moved again (see beginning of this function)

interpreter.invalidateReferencedResources(v, false)

if config.InvalidatedResourceValidationEnabled {
v.dictionary = nil
} else {
v.dictionary = dictionary
res = v
}

newStorageID := dictionary.StorageID()

interpreter.updateReferencedResource(
currentStorageID,
newStorageID,
func(value ReferenceTrackedResourceKindedValue) {
compositeValue, ok := value.(*CompositeValue)
if !ok {
panic(errors.NewUnreachableError())
}

// Any kind of move would invalidate the references.
compositeValue.dictionary = nil
},
)
}

if res == nil {
Expand Down Expand Up @@ -17265,8 +17218,19 @@ func (v *CompositeValue) GetOwner() common.Address {
// ForEachField iterates over all field-name field-value pairs of the composite value.
// It does NOT iterate over computed fields and functions!
func (v *CompositeValue) ForEachField(gauge common.MemoryGauge, f func(fieldName string, fieldValue Value)) {
v.forEachField(gauge, v.dictionary.Iterate, f)
}

func (v *CompositeValue) ForEachLoadedField(gauge common.MemoryGauge, f func(fieldName string, fieldValue Value)) {
v.forEachField(gauge, v.dictionary.IterateLoadedValues, f)
}

err := v.dictionary.Iterate(func(key atree.Value, value atree.Value) (resume bool, err error) {
func (v *CompositeValue) forEachField(
gauge common.MemoryGauge,
atreeIterate func(fn atree.MapEntryIterationFunc) error,
f func(fieldName string, fieldValue Value),
) {
err := atreeIterate(func(key atree.Value, value atree.Value) (resume bool, err error) {
f(
string(key.(StringAtreeValue)),
MustConvertStoredValue(gauge, value),
Expand Down Expand Up @@ -17816,8 +17780,20 @@ func (v *DictionaryValue) Accept(interpreter *Interpreter, visitor Visitor) {
}

func (v *DictionaryValue) Iterate(interpreter *Interpreter, f func(key, value Value) (resume bool)) {
v.iterate(interpreter, v.dictionary.Iterate, f)
}

func (v *DictionaryValue) IterateLoaded(interpreter *Interpreter, f func(key, value Value) (resume bool)) {
v.iterate(interpreter, v.dictionary.IterateLoadedValues, f)
}

func (v *DictionaryValue) iterate(
interpreter *Interpreter,
atreeIterate func(fn atree.MapEntryIterationFunc) error,
f func(key Value, value Value) (resume bool),
) {
iterate := func() {
err := v.dictionary.Iterate(func(key, value atree.Value) (resume bool, err error) {
err := atreeIterate(func(key, value atree.Value) (resume bool, err error) {
// atree.OrderedMap iteration provides low-level atree.Value,
// convert to high-level interpreter.Value

Expand Down Expand Up @@ -17953,26 +17929,11 @@ func (v *DictionaryValue) Destroy(interpreter *Interpreter, locationRange Locati

v.isDestroyed = true

interpreter.invalidateReferencedResources(v, true)

if config.InvalidatedResourceValidationEnabled {
v.dictionary = nil
}

interpreter.updateReferencedResource(
storageID,
storageID,
func(value ReferenceTrackedResourceKindedValue) {
dictionaryValue, ok := value.(*DictionaryValue)
if !ok {
panic(errors.NewUnreachableError())
}

dictionaryValue.isDestroyed = true

if config.InvalidatedResourceValidationEnabled {
dictionaryValue.dictionary = nil
}
},
)
}

func (v *DictionaryValue) ForEachKey(
Expand Down Expand Up @@ -18793,28 +18754,14 @@ func (v *DictionaryValue) Transfer(
// This allows raising an error when the resource array is attempted
// to be transferred/moved again (see beginning of this function)

interpreter.invalidateReferencedResources(v, false)

if config.InvalidatedResourceValidationEnabled {
v.dictionary = nil
} else {
v.dictionary = dictionary
res = v
}

newStorageID := dictionary.StorageID()

interpreter.updateReferencedResource(
currentStorageID,
newStorageID,
func(value ReferenceTrackedResourceKindedValue) {
dictionaryValue, ok := value.(*DictionaryValue)
if !ok {
panic(errors.NewUnreachableError())
}

// Any kind of move would invalidate the references.
dictionaryValue.dictionary = nil
},
)
}

if res == nil {
Expand Down
Loading

0 comments on commit 341b7b5

Please sign in to comment.