Remove a few code spots with potential to deadlock (#2811)

to quickly summarize, our logging library (seelog) has the potential to
deadlock if a task update happens at the same time as it is trying to
unmarshal the task object into a string.

long-term we are working to replace this library, but for now we can
remove the potential to trigger this deadlock by removing the lock on
Task.String(), and remove an unecessary Debug log statement in the
AddStateChangeEvent function.

- remove lock from Task.String
- AddStateChangeEvent: remove log.Debugf statement

Makefile

@@ -278,7 +278,7 @@ static-check: gocyclo govet importcheck gogenerate-check
 	# use default checks of staticcheck tool, except style checks (-ST*) and depracation checks (-SA1019)
 	# depracation checks have been left out for now; removing their warnings requires error handling for newer suggested APIs, changes in function signatures and their usages.
 	# https://github.com/dominikh/go-tools/tree/master/cmd/staticcheck
-	staticcheck -tests=false -checks "inherit,-ST*,-SA1019" ./agent/...
+	staticcheck -tests=false -checks "inherit,-ST*,-SA1019,-SA9002" ./agent/...
 
 .PHONY: goimports
 goimports:

agent/api/task/task.go

@@ -2247,35 +2247,15 @@ func (task *Task) GetExecutionStoppedAt() time.Time {
 
 // String returns a human readable string representation of this object
 func (task *Task) String() string {
-	task.lock.Lock()
-	defer task.lock.Unlock()
 	return task.stringUnsafe()
 }
 
 // stringUnsafe returns a human readable string representation of this object
 func (task *Task) stringUnsafe() string {
-	res := fmt.Sprintf("%s:%s %s, TaskStatus: (%s->%s)",
+	return fmt.Sprintf("%s:%s %s, TaskStatus: (%s->%s) N Containers: %d, N ENIs %d",
 		task.Family, task.Version, task.Arn,
-		task.KnownStatusUnsafe.String(), task.DesiredStatusUnsafe.String())
-
-	res += " Containers: ["
-	for _, container := range task.Containers {
-		res += fmt.Sprintf("%s (%s->%s),",
-			container.Name,
-			container.GetKnownStatus().String(),
-			container.GetDesiredStatus().String())
-	}
-	res += "]"
-
-	if len(task.ENIs) > 0 {
-		res += " ENIs: ["
-		for _, eni := range task.ENIs {
-			res += fmt.Sprintf("%s,", eni.String())
-		}
-		res += "]"
-	}
-
-	return res
+		task.KnownStatusUnsafe.String(), task.DesiredStatusUnsafe.String(),
+		len(task.Containers), len(task.ENIs))
 }
 
 // GetID is used to retrieve the taskID from taskARN

agent/eventhandler/task_handler.go

@@ -133,7 +133,6 @@ func NewTaskHandler(ctx context.Context,
 func (handler *TaskHandler) AddStateChangeEvent(change statechange.Event, client api.ECSClient) error {
 	handler.lock.Lock()
 	defer handler.lock.Unlock()
-	seelog.Debugf("handling Event: %v", change)
 	switch change.GetEventType() {
 	case statechange.TaskEvent:
 		event, ok := change.(api.TaskStateChange)