fatal error: concurrent map writes when u

[delanne]

Expected Behavior

• no crash

Actual Behavior

when testing several Activities with t.Parallel, the test framework crash with "fatal error: concurrent map writes"

fatal error: concurrent map writes
...
goroutine 129 [running]:
go.temporal.io/sdk/internal.(*testWorkflowEnvironmentImpl).setActivityHandle(...)
    /Users/xavier.delannoy/go/pkg/mod/go.temporal.io/sdk@v1.30.0/internal/internal_workflow_testsuite.go:1415
go.temporal.io/sdk/internal.(*testWorkflowEnvironmentImpl).executeActivity(0x140000e4488, {0x10304e900?, 0x1031d92a8?}, {0x14000513020, 0x1, 0x1})
    /Users/xavier.delannoy/go/pkg/mod/go.temporal.io/sdk@v1.30.0/internal/internal_workflow_testsuite.go:703 +0x524
go.temporal.io/sdk/internal.(*TestActivityEnvironment).ExecuteActivity(0x1030c99a0?, {0x10304e900?, 0x1031d92a8?}, {0x14000513020?, 0x140004d1f38?, 0x1028bb6a0?})
    /Users/xavier.delannoy/go/pkg/mod/go.temporal.io/sdk@v1.30.0/internal/workflow_testsuite.go:195 +0x2c
github.com/strangebee/godsl/flow/activities/strings.Test_Activity_TrimRight.func1(0x140004c36c0)
    /Users/xavier.delannoy/go/src/github.com/strangebee/Delannoy/godsl/flow/activities/strings/trimright_test.go:150 +0xac
testing.tRunner(0x140004c36c0, 0x14000494390)
    /usr/local/go/src/testing/testing.go:1690 +0xe4
created by testing.(*T).Run in goroutine 24
    /usr/local/go/src/testing/testing.go:1743 +0x314

Steps to Reproduce the Problem

1. write some activities
2. write tests for these activities with t.Parallel
3. run tests in a loop in order to reproduce the race conditions

Specifications

• Version: SDK 1.30
• Platform: Darwin

Investigation

code:

func (env *testWorkflowEnvironmentImpl) setActivityHandle(activityID, runID string, handle *testActivityHandle) {
    env.activities[env.makeUniqueActivityID(activityID, runID)] = handle
}

        activities             map[string]*testActivityHandle

as several go routines can write in the map 'activities', the type of activities should be a sync.Map or use a mutex to protect the write

Potential fix

internal/internal_workflow_testsuite.go:1415

func (env *testWorkflowEnvironmentImpl) setActivityHandle(activityID, runID string, handle *testActivityHandle) {
    env.locker.Lock()
    defer env.locker.Unlock()
    env.activities[env.makeUniqueActivityID(activityID, runID)] = handle
}

[Quinn-With-Two-Ns]

To clarify, are you seeing this data race when the same test environment being shared by multiple tests?

[delanne]

To clarify, are you seeing this data race when the same test environment being shared by multiple tests?

all tests are run in parallel. Here is an example of how I write a test

func Test_Activity_FooBar(t *testing.T) {
    testSuite := &testsuite.WorkflowTestSuite{}
    env := testSuite.NewTestActivityEnvironment()

    env.RegisterActivity(FooBar)

    t.Parallel()
    tests := []struct {
        name     string
        p        *FooBarParams
        expected internal.Return
    }{
        {
            name: "foobar test1",
            p: &FooBarParams{
                String1: "foobar",
                String2: "foobar",
            },
            expected: internal.Return{Value: float64(0)},
        },
     .....
    }

    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            t.Parallel()
            val, err := env.ExecuteActivity(FooBar,
                tt.p,
            )
            require.NoError(t, err)
            res := &internal.Return{}
            err = val.Get(res)
            require.NoError(t, err)
            require.Equal(t, &tt.expected, res)
        })
    }
}

Do you mean that the line env := testSuite.NewTestActivityEnvironment() should be after the line t.Parallel() ? (make sense to me)