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

Invalidate references to nested resources upon the move of the outer resource #2460

Merged
2 changes: 1 addition & 1 deletion runtime/attachments_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ func TestAccountAttachmentExportFailure(t *testing.T) {
},
)
require.Error(t, err)
require.ErrorAs(t, err, &interpreter.DestroyedResourceError{})
require.ErrorAs(t, err, &interpreter.InvalidatedResourceReferenceError{})
}

func TestAccountAttachmentExport(t *testing.T) {
Expand Down
64 changes: 51 additions & 13 deletions runtime/interpreter/interpreter.go
Original file line number Diff line number Diff line change
Expand Up @@ -4657,25 +4657,63 @@ 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) {
// skip non-resource typed values
if !value.IsResourceKinded(interpreter) {
return
}

var storageID atree.StorageID

switch value := value.(type) {
case *CompositeValue:
value.ForEachField(interpreter, func(_ string, fieldValue Value) {
interpreter.invalidateReferencedResources(fieldValue)
})
storageID = value.StorageID()
case *DictionaryValue:
value.Iterate(interpreter, func(_, value Value) (resume bool) {
interpreter.invalidateReferencedResources(value)
return true
})
storageID = value.StorageID()
case *ArrayValue:
value.Iterate(interpreter, func(element Value) (resume bool) {
interpreter.invalidateReferencedResources(element)
return true
})
storageID = value.StorageID()
case *SomeValue:
interpreter.invalidateReferencedResources(value.value)
return
default:
// skip non-container typed values.
return
SupunS marked this conversation as resolved.
Show resolved Hide resolved
}

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
case *DictionaryValue:
value.dictionary = nil
case *ArrayValue:
value.array = nil
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
117 changes: 12 additions & 105 deletions runtime/interpreter/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -1608,8 +1608,6 @@ func (v *ArrayValue) Destroy(interpreter *Interpreter, locationRange LocationRan
v.checkInvalidatedResourceUse(interpreter, locationRange)
}

storageID := v.StorageID()

if config.TracingEnabled {
startTime := time.Now()

Expand All @@ -1631,26 +1629,11 @@ func (v *ArrayValue) Destroy(interpreter *Interpreter, locationRange LocationRan

v.isDestroyed = true

interpreter.invalidateReferencedResources(v)

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 @@ -2474,28 +2457,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)

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 @@ -14046,8 +14015,6 @@ func (v *CompositeValue) Destroy(interpreter *Interpreter, locationRange Locatio
v.checkInvalidatedResourceUse(interpreter, locationRange)
}

storageID := v.StorageID()

if config.TracingEnabled {
startTime := time.Now()

Expand Down Expand Up @@ -14108,26 +14075,11 @@ func (v *CompositeValue) Destroy(interpreter *Interpreter, locationRange Locatio

v.isDestroyed = true

interpreter.invalidateReferencedResources(v)

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 @@ -14856,28 +14808,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)

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 @@ -15486,8 +15424,6 @@ func (v *DictionaryValue) Destroy(interpreter *Interpreter, locationRange Locati
v.checkInvalidatedResourceUse(interpreter, locationRange)
}

storageID := v.StorageID()

if config.TracingEnabled {
startTime := time.Now()

Expand All @@ -15512,26 +15448,11 @@ func (v *DictionaryValue) Destroy(interpreter *Interpreter, locationRange Locati

v.isDestroyed = true

interpreter.invalidateReferencedResources(v)

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 @@ -16308,28 +16229,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)

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
4 changes: 2 additions & 2 deletions runtime/tests/interpreter/attachments_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1624,7 +1624,7 @@ func TestInterpretAttachmentResourceReferenceInvalidation(t *testing.T) {
)

_, err := inter.Invoke("test")
require.ErrorAs(t, err, &interpreter.DestroyedResourceError{})
require.ErrorAs(t, err, &interpreter.InvalidatedResourceReferenceError{})
})

t.Run("nested", func(t *testing.T) {
Expand Down Expand Up @@ -1748,7 +1748,7 @@ func TestInterpretAttachmentResourceReferenceInvalidation(t *testing.T) {
)

_, err := inter.Invoke("test")
require.ErrorAs(t, err, &interpreter.DestroyedResourceError{})
require.ErrorAs(t, err, &interpreter.InvalidatedResourceReferenceError{})
})

t.Run("self reference", func(t *testing.T) {
Expand Down
Loading