From 2bceb132bac42880da164e6d26480bb9e6c4dda1 Mon Sep 17 00:00:00 2001 From: Shijie Sheng Date: Wed, 11 Dec 2024 16:24:26 -0800 Subject: [PATCH] Fix incorrect nil handling in workflowTaskPoller (#1412) What changed? explicit nil handling in newWorkflowTaskPoller for ldaTunnel Why? Golang doesn't handle passing in nil pointer of concrete type directly into the interface. It still considers it not nil, which will cause panic in this case. How did you test it? Unit test fail before and pass after the change --- internal/internal_task_pollers.go | 10 +++++++++- internal/internal_task_pollers_test.go | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/internal/internal_task_pollers.go b/internal/internal_task_pollers.go index 3b8a2e6c0..a69b8765d 100644 --- a/internal/internal_task_pollers.go +++ b/internal/internal_task_pollers.go @@ -290,6 +290,14 @@ func newWorkflowTaskPoller( domain string, params workerExecutionParameters, ) *workflowTaskPoller { + /* + Explicit nil handling is needed here. Otherwise, poller.ldaTunnel would not return a nil result + */ + var ldaTunnelInterface localDispatcher + if ldaTunnel != nil { + ldaTunnelInterface = ldaTunnel + } + return &workflowTaskPoller{ basePoller: basePoller{shutdownC: params.WorkerStopChannel}, service: service, @@ -297,7 +305,7 @@ func newWorkflowTaskPoller( taskListName: params.TaskList, identity: params.Identity, taskHandler: taskHandler, - ldaTunnel: ldaTunnel, + ldaTunnel: ldaTunnelInterface, metricsScope: metrics.NewTaggedScope(params.MetricsScope), logger: params.Logger, stickyUUID: uuid.New(), diff --git a/internal/internal_task_pollers_test.go b/internal/internal_task_pollers_test.go index ef73f3139..ed0a4e779 100644 --- a/internal/internal_task_pollers_test.go +++ b/internal/internal_task_pollers_test.go @@ -46,6 +46,21 @@ const ( _testIdentity = "test-worker" ) +func Test_newWorkflowTaskPoller(t *testing.T) { + t.Run("success with nil ldaTunnel", func(t *testing.T) { + poller := newWorkflowTaskPoller( + nil, + nil, + nil, + _testDomainName, + workerExecutionParameters{}) + assert.NotNil(t, poller) + if poller.ldaTunnel != nil { + t.Error("unexpected not nil ldaTunnel") + } + }) +} + func TestLocalActivityPanic(t *testing.T) { // regression: panics in local activities should not terminate the process s := WorkflowTestSuite{logger: testlogger.NewZap(t)}