TestMetricChanPush panics on 386 and arm

[anthonyfok]

Version of AWS SDK for Go?

1.16.18, 1.21.6 and master

Version of Go (go version)?

• go version go1.12.9 linux/386
• go version go1.12.9 linux/arm

What issue did you see?

Ubuntu’s autopkgtest detects a regression on 386 (i386) and arm (armhf) when trying to upgrading golang-github-aws-aws-go from 1.4.22 to 1.16.18 or to 1.21.6, see https://autopkgtest.ubuntu.com/packages/golang-github-aws-aws-sdk-go

386:

=== RUN   TestMetricChanPush
--- FAIL: TestMetricChanPush (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x804aa4b]

goroutine 15 [running]:
testing.tRunner.func1(0xa0aa780)
	/usr/lib/go-1.12/src/testing/testing.go:830 +0x345
panic(0x83ba0e0, 0x8726528)
	/usr/lib/go-1.12/src/runtime/panic.go:522 +0x16e
runtime/internal/atomic.Load64(0xa00e6f4, 0xffffffe1, 0x84bddfd)
	/usr/lib/go-1.12/src/runtime/internal/atomic/asm_386.s:194 +0xb
github.com/aws/aws-sdk-go/aws/csm.(*metricChan).IsPaused(...)
	/tmp/autopkgtest.JPS3Pq/autopkgtest_tmp/obj-i686-linux-gnu/src/github.com/aws/aws-sdk-go/aws/csm/metric_chan.go:37
github.com/aws/aws-sdk-go/aws/csm.(*metricChan).Push(0xa00e6f0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/tmp/autopkgtest.JPS3Pq/autopkgtest_tmp/obj-i686-linux-gnu/src/github.com/aws/aws-sdk-go/aws/csm/metric_chan.go:44 +0x35
github.com/aws/aws-sdk-go/aws/csm.TestMetricChanPush(0xa0aa780)
	/tmp/autopkgtest.JPS3Pq/autopkgtest_tmp/obj-i686-linux-gnu/src/github.com/aws/aws-sdk-go/aws/csm/metric_chan_test.go:11 +0xb9
testing.tRunner(0xa0aa780, 0x8422234)
	/usr/lib/go-1.12/src/testing/testing.go:865 +0x97
created by testing.(*T).Run
	/usr/lib/go-1.12/src/testing/testing.go:916 +0x2b5
FAIL	github.com/aws/aws-sdk-go/aws/csm	0.011s

arm:

=== RUN   TestMetricChanPush
--- FAIL: TestMetricChanPush (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x129cc]

goroutine 15 [running]:
testing.tRunner.func1(0x1ca4780)
	/usr/lib/go-1.12/src/testing/testing.go:830 +0x364
panic(0x39b850, 0x6fb7b8)
	/usr/lib/go-1.12/src/runtime/panic.go:522 +0x1a0
runtime/internal/atomic.goLoad64(0x1c0e6d4, 0xe7502200, 0x1c6c8)
	/usr/lib/go-1.12/src/runtime/internal/atomic/atomic_arm.go:127 +0x1c
github.com/aws/aws-sdk-go/aws/csm.(*metricChan).IsPaused(...)
	/tmp/autopkgtest.8X8CPq/autopkgtest_tmp/obj-arm-linux-gnueabihf/src/github.com/aws/aws-sdk-go/aws/csm/metric_chan.go:37
github.com/aws/aws-sdk-go/aws/csm.(*metricChan).Push(0x1c0e6d0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/tmp/autopkgtest.8X8CPq/autopkgtest_tmp/obj-arm-linux-gnueabihf/src/github.com/aws/aws-sdk-go/aws/csm/metric_chan.go:44 +0x2c
github.com/aws/aws-sdk-go/aws/csm.TestMetricChanPush(0x1ca4780)
	/tmp/autopkgtest.8X8CPq/autopkgtest_tmp/obj-arm-linux-gnueabihf/src/github.com/aws/aws-sdk-go/aws/csm/metric_chan_test.go:11 +0xa4
testing.tRunner(0x1ca4780, 0x3feacc)
	/usr/lib/go-1.12/src/testing/testing.go:865 +0xa4
created by testing.(*T).Run
	/usr/lib/go-1.12/src/testing/testing.go:916 +0x2b8
FAIL	github.com/aws/aws-sdk-go/aws/csm	0.075s

[jasdel]

Thanks for reaching out to us and creating the PR. Reviewing the newMetricChan code I think the root issue is that metricChan struct field, paused is being used in an unsafe atomic operation. The metricChan value should be a pointer since any copy of that structure will cause paused address to change making the atomic operations invalid.

I think we need to update newMetricChan to return a pointer instead of value. Or alternatively the paused member should be a pointer instead of value.

type metricChan struct {
	paused *int64
	ch     chan metric
}

func newMetricChan(size int) metricChan {
	return metricChan{
		paused: new(int),
		ch: make(chan metric, size),
	}
}

func (ch *metricChan) Pause() {
	atomic.StoreInt64(ch.paused, pausedEnum)
}

func (ch *metricChan) Continue() {
	atomic.StoreInt64(ch.paused, runningEnum)
}

func (ch *metricChan) IsPaused() bool {
	v := atomic.LoadInt64(ch.paused)
	return v == pausedEnum
}

[anthonyfok]

Thank you so much for explaining to me the root of the issue and for showing me the proper way to fix it. I have just force pushed a new commit to #2785, which is essentially what you showed me in the code above. Many thanks!