From 6cd32aa0a60e7b1332f6a759e58a1981069005b1 Mon Sep 17 00:00:00 2001
From: Cameron Sparr <sparrc@users.noreply.github.com>
Date: Thu, 18 Feb 2021 23:51:55 -0800
Subject: [PATCH] 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                           |  2 +-
 agent/api/task/task.go             | 26 +++-----------------------
 agent/eventhandler/task_handler.go |  1 -
 3 files changed, 4 insertions(+), 25 deletions(-)

diff --git a/Makefile b/Makefile
index 8cb7b2b340a..727d7cd1f24 100644
--- a/Makefile
+++ b/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:
diff --git a/agent/api/task/task.go b/agent/api/task/task.go
index 04ada154b11..5f91d302af9 100644
--- a/agent/api/task/task.go
+++ b/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
diff --git a/agent/eventhandler/task_handler.go b/agent/eventhandler/task_handler.go
index 13f3386682e..9301ffbb12d 100644
--- a/agent/eventhandler/task_handler.go
+++ b/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)