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

Improve resource-reference tracking #2958

Merged
merged 12 commits into from
Dec 5, 2023
33 changes: 16 additions & 17 deletions runtime/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,23 +168,22 @@ func newInterpreterEnvironment(config Config) *interpreterEnvironment {

func (e *interpreterEnvironment) newInterpreterConfig() *interpreter.Config {
return &interpreter.Config{
InvalidatedResourceValidationEnabled: true,
MemoryGauge: e,
BaseActivationHandler: e.getBaseActivation,
OnEventEmitted: e.newOnEventEmittedHandler(),
OnAccountLinked: e.newOnAccountLinkedHandler(),
InjectedCompositeFieldsHandler: e.newInjectedCompositeFieldsHandler(),
UUIDHandler: e.newUUIDHandler(),
ContractValueHandler: e.newContractValueHandler(),
ImportLocationHandler: e.newImportLocationHandler(),
PublicAccountHandler: e.newPublicAccountHandler(),
AuthAccountHandler: e.newAuthAccountHandler(),
OnRecordTrace: e.newOnRecordTraceHandler(),
OnResourceOwnerChange: e.newResourceOwnerChangedHandler(),
CompositeTypeHandler: e.newCompositeTypeHandler(),
CompositeValueFunctionsHandler: e.newCompositeValueFunctionsHandler(),
TracingEnabled: e.config.TracingEnabled,
AtreeValueValidationEnabled: e.config.AtreeValidationEnabled,
MemoryGauge: e,
BaseActivationHandler: e.getBaseActivation,
OnEventEmitted: e.newOnEventEmittedHandler(),
OnAccountLinked: e.newOnAccountLinkedHandler(),
InjectedCompositeFieldsHandler: e.newInjectedCompositeFieldsHandler(),
UUIDHandler: e.newUUIDHandler(),
ContractValueHandler: e.newContractValueHandler(),
ImportLocationHandler: e.newImportLocationHandler(),
PublicAccountHandler: e.newPublicAccountHandler(),
AuthAccountHandler: e.newAuthAccountHandler(),
OnRecordTrace: e.newOnRecordTraceHandler(),
OnResourceOwnerChange: e.newResourceOwnerChangedHandler(),
CompositeTypeHandler: e.newCompositeTypeHandler(),
CompositeValueFunctionsHandler: e.newCompositeValueFunctionsHandler(),
TracingEnabled: e.config.TracingEnabled,
AtreeValueValidationEnabled: e.config.AtreeValidationEnabled,
// NOTE: ignore e.config.AtreeValidationEnabled here,
// and disable storage validation after each value modification.
// Instead, storage is validated after commits (if validation is enabled),
Expand Down
2 changes: 0 additions & 2 deletions runtime/interpreter/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@ type Config struct {
OnStatement OnStatementFunc
// OnLoopIteration is triggered when a loop iteration is about to be executed
OnLoopIteration OnLoopIterationFunc
// InvalidatedResourceValidationEnabled determines if the validation of invalidated resources is enabled
InvalidatedResourceValidationEnabled bool
// TracingEnabled determines if tracing is enabled.
// Tracing reports certain operations, e.g. composite value transfers
TracingEnabled bool
Expand Down
31 changes: 10 additions & 21 deletions runtime/interpreter/interpreter.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ type Storage interface {
CheckHealth() error
}

type ReferencedResourceKindedValues map[atree.StorageID]map[ReferenceTrackedResourceKindedValue]struct{}
type ReferencedResourceKindedValues map[atree.StorageID]map[*EphemeralReferenceValue]struct{}
SupunS marked this conversation as resolved.
Show resolved Hide resolved

type Interpreter struct {
Location common.Location
Expand Down Expand Up @@ -838,14 +838,12 @@ func (interpreter *Interpreter) resultValue(returnValue Value, returnType sema.T
optionalType.Type,
)

interpreter.maybeTrackReferencedResourceKindedValue(returnValue.value)
return NewSomeValueNonCopying(interpreter, innerValue)
case NilValue:
return NilValue{}
}
}

interpreter.maybeTrackReferencedResourceKindedValue(returnValue)
return NewEphemeralReferenceValue(interpreter, false, returnValue, returnType)
}

Expand Down Expand Up @@ -5260,18 +5258,20 @@ func (interpreter *Interpreter) ValidateAtreeValue(value atree.Value) {
}

func (interpreter *Interpreter) maybeTrackReferencedResourceKindedValue(value Value) {
if value, ok := value.(ReferenceTrackedResourceKindedValue); ok {
interpreter.trackReferencedResourceKindedValue(value.StorageID(), value)
if referenceValue, ok := value.(*EphemeralReferenceValue); ok {
if value, ok := referenceValue.Value.(ReferenceTrackedResourceKindedValue); ok {
interpreter.trackReferencedResourceKindedValue(value.StorageID(), referenceValue)
}
}
}

func (interpreter *Interpreter) trackReferencedResourceKindedValue(
id atree.StorageID,
value ReferenceTrackedResourceKindedValue,
value *EphemeralReferenceValue,
) {
values := interpreter.SharedState.referencedResourceKindedValues[id]
if values == nil {
values = map[ReferenceTrackedResourceKindedValue]struct{}{}
values = map[*EphemeralReferenceValue]struct{}{}
interpreter.SharedState.referencedResourceKindedValues[id] = values
}
values[value] = struct{}{}
Expand All @@ -5280,7 +5280,7 @@ func (interpreter *Interpreter) trackReferencedResourceKindedValue(
func (interpreter *Interpreter) updateReferencedResource(
currentStorageID atree.StorageID,
newStorageID atree.StorageID,
updateFunc func(value ReferenceTrackedResourceKindedValue),
updateFunc func(value *EphemeralReferenceValue),
) {
values := interpreter.SharedState.referencedResourceKindedValues[currentStorageID]
if values == nil {
Expand All @@ -5304,10 +5304,7 @@ func (interpreter *Interpreter) startResourceTracking(
hasPosition ast.HasPosition,
) {

config := interpreter.SharedState.Config

if !config.InvalidatedResourceValidationEnabled ||
identifier == sema.SelfIdentifier {
if identifier == sema.SelfIdentifier {
return
}

Expand Down Expand Up @@ -5339,10 +5336,8 @@ func (interpreter *Interpreter) checkInvalidatedResourceUse(
identifier string,
hasPosition ast.HasPosition,
) {
config := interpreter.SharedState.Config

if !config.InvalidatedResourceValidationEnabled ||
identifier == sema.SelfIdentifier {
if identifier == sema.SelfIdentifier {
return
}

Expand Down Expand Up @@ -5385,12 +5380,6 @@ func (interpreter *Interpreter) resourceForValidation(value Value) ResourceKinde
}

func (interpreter *Interpreter) invalidateResource(value Value) {
config := interpreter.SharedState.Config

if !config.InvalidatedResourceValidationEnabled {
return
}

if value == nil || !value.IsResourceKinded(interpreter) {
return
}
Expand Down
35 changes: 16 additions & 19 deletions runtime/interpreter/interpreter_expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,19 @@ func (interpreter *Interpreter) VisitIdentifierExpression(expression *ast.Identi
}

func (interpreter *Interpreter) evalExpression(expression ast.Expression) Value {
return ast.AcceptExpression[Value](expression, interpreter)
result := ast.AcceptExpression[Value](expression, interpreter)

resourceKindedValue, ok := result.(ResourceKindedValue)
if ok && resourceKindedValue.isInvalidatedResource(interpreter) {
panic(InvalidatedResourceError{
LocationRange: LocationRange{
Location: interpreter.Location,
HasPosition: expression,
},
})
}
SupunS marked this conversation as resolved.
Show resolved Hide resolved

return result
}

func (interpreter *Interpreter) VisitBinaryExpression(expression *ast.BinaryExpression) Value {
Expand Down Expand Up @@ -960,15 +972,6 @@ func (interpreter *Interpreter) visitInvocationExpressionWithImplicitArgument(in
panic(errors.NewUnreachableError())
}

// Bound functions
if boundFunction, ok := function.(BoundFunctionValue); ok && boundFunction.Self != nil {
self := *boundFunction.Self
if resource, ok := self.(ReferenceTrackedResourceKindedValue); ok {
storageID := resource.StorageID()
interpreter.trackReferencedResourceKindedValue(storageID, resource)
}
}

SupunS marked this conversation as resolved.
Show resolved Hide resolved
// NOTE: evaluate all argument expressions in call-site scope, not in function body

var argumentExpressions []ast.Expression
Expand Down Expand Up @@ -1183,8 +1186,6 @@ func (interpreter *Interpreter) VisitReferenceExpression(referenceExpression *as

result := interpreter.evalExpression(referenceExpression.Expression)

interpreter.maybeTrackReferencedResourceKindedValue(result)

// There are four potential cases:
// 1) Target type is optional, actual value is also optional (nil/SomeValue)
// 2) Target type is optional, actual value is non-optional
Expand All @@ -1211,7 +1212,6 @@ func (interpreter *Interpreter) VisitReferenceExpression(referenceExpression *as
}

innerValue := result.InnerValue(interpreter, locationRange)
interpreter.maybeTrackReferencedResourceKindedValue(innerValue)

return NewSomeValueNonCopying(
interpreter,
Expand Down Expand Up @@ -1251,14 +1251,15 @@ func (interpreter *Interpreter) VisitReferenceExpression(referenceExpression *as

case *sema.ReferenceType:
// Case (3): target type is non-optional, actual value is optional.
// Unwrap the optional and add it to reference tracking.
// This path shouldn't be reachable. This is only a defensive step
// to ensure references are properly created/tracked.
if someValue, ok := result.(*SomeValue); ok {
locationRange := LocationRange{
Location: interpreter.Location,
HasPosition: referenceExpression.Expression,
}
innerValue := someValue.InnerValue(interpreter, locationRange)
interpreter.maybeTrackReferencedResourceKindedValue(innerValue)
return NewEphemeralReferenceValue(interpreter, typ.Authorized, innerValue, typ.Type)
SupunS marked this conversation as resolved.
Show resolved Hide resolved
}

// Case (4): target type is non-optional, actual value is also non-optional
Expand Down Expand Up @@ -1341,7 +1342,6 @@ func (interpreter *Interpreter) VisitAttachExpression(attachExpression *ast.Atta
base,
interpreter.MustSemaTypeOfValue(base).(*sema.CompositeType),
)
interpreter.trackReferencedResourceKindedValue(base.StorageID(), base)

attachment, ok := interpreter.visitInvocationExpressionWithImplicitArgument(
attachExpression.Attachment,
Expand All @@ -1352,9 +1352,6 @@ func (interpreter *Interpreter) VisitAttachExpression(attachExpression *ast.Atta
panic(errors.NewUnreachableError())
}

// Because `self` in attachments is a reference, we need to track the attachment if it's a resource
interpreter.trackReferencedResourceKindedValue(attachment.StorageID(), attachment)

base = base.Transfer(
interpreter,
locationRange,
Expand Down
2 changes: 1 addition & 1 deletion runtime/interpreter/sharedstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func NewSharedState(config *Config) *SharedState {
},
inStorageIteration: false,
storageMutatedDuringIteration: false,
referencedResourceKindedValues: map[atree.StorageID]map[ReferenceTrackedResourceKindedValue]struct{}{},
referencedResourceKindedValues: map[atree.StorageID]map[*EphemeralReferenceValue]struct{}{},
resourceVariables: map[ResourceKindedValue]*Variable{},
CapabilityControllerIterations: map[AddressPath]int{},
containerValueIteration: map[atree.StorageID]struct{}{},
Expand Down
Loading
Loading