Skip to content

Commit

Permalink
improving obviously-unsafe shallow copy
Browse files Browse the repository at this point in the history
  • Loading branch information
Groxx committed Dec 20, 2024
1 parent b22774a commit ed17043
Showing 1 changed file with 34 additions and 5 deletions.
39 changes: 34 additions & 5 deletions service/history/execution/mutable_state_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ func CopyWorkflowExecutionInfo(sourceInfo *persistence.WorkflowExecutionInfo) *p
IsCron: sourceInfo.IsCron,
InitiatedID: sourceInfo.InitiatedID,
CompletionEventBatchID: sourceInfo.CompletionEventBatchID,
CompletionEvent: sourceInfo.CompletionEvent,
CompletionEvent: sourceInfo.CompletionEvent, // not deep-copied, history events aren't mutated (unverified but seems very unlikely)
TaskList: sourceInfo.TaskList,
StickyTaskList: sourceInfo.StickyTaskList,
StickyScheduleToStartTimeout: sourceInfo.StickyScheduleToStartTimeout,
Expand Down Expand Up @@ -378,10 +378,10 @@ func CopyWorkflowExecutionInfo(sourceInfo *persistence.WorkflowExecutionInfo) *p
ClientLibraryVersion: sourceInfo.ClientLibraryVersion,
ClientFeatureVersion: sourceInfo.ClientFeatureVersion,
ClientImpl: sourceInfo.ClientImpl,
AutoResetPoints: sourceInfo.AutoResetPoints,
Memo: sourceInfo.Memo,
SearchAttributes: sourceInfo.SearchAttributes,
PartitionConfig: sourceInfo.PartitionConfig,
AutoResetPoints: copyResetPoints(sourceInfo.AutoResetPoints), // points slice is deep-copied, to prevent appends from mutating the contents
Memo: copyMap(sourceInfo.Memo), // memo bytes value not deep-copied, but they are not mutated in place
SearchAttributes: copyMap(sourceInfo.SearchAttributes), // attr bytes value not deep-copied, but they are not mutated in place
PartitionConfig: copyMap(sourceInfo.PartitionConfig),
Attempt: sourceInfo.Attempt,
HasRetryPolicy: sourceInfo.HasRetryPolicy,
InitialInterval: sourceInfo.InitialInterval,
Expand All @@ -395,6 +395,35 @@ func CopyWorkflowExecutionInfo(sourceInfo *persistence.WorkflowExecutionInfo) *p
}
}

func copyResetPoints(orig *types.ResetPoints) *types.ResetPoints {
if orig == nil {
return nil
}
if len(orig.Points) == 0 {
// tests imply this branch is relevant, but unverified
return &types.ResetPoints{
Points: nil,
}
}
// +1 cap because an append is expected when doing a reset or continue-as-new
points := make([]*types.ResetPointInfo, len(orig.Points), len(orig.Points)+1)
copy(points, orig.Points)
return &types.ResetPoints{
Points: points,
}
}

func copyMap[K comparable, V any](orig map[K]V) map[K]V {
if orig == nil {
return nil
}
dup := make(map[K]V, len(orig))
for k, v := range orig {
dup[k] = v
}
return dup
}

// CopyActivityInfo copies ActivityInfo
func CopyActivityInfo(sourceInfo *persistence.ActivityInfo) *persistence.ActivityInfo {
details := slices.Clone(sourceInfo.Details)
Expand Down

0 comments on commit ed17043

Please sign in to comment.