diff --git a/.changelog/10515.txt b/.changelog/10515.txt new file mode 100644 index 0000000000..b83e5e154e --- /dev/null +++ b/.changelog/10515.txt @@ -0,0 +1,3 @@ +```release-note:bug +agent: fix a panic on 32-bit platforms caused by misaligned struct fields used with sync/atomic. +``` diff --git a/.circleci/config.yml b/.circleci/config.yml index 1b1d54120e..5a273c4804 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -116,14 +116,20 @@ jobs: - run: go get -u github.com/hashicorp/lint-consul-retry && lint-consul-retry - run: *notify-slack-failure - # Runs Go linters lint: + description: "Run golangci-lint" + parameters: + go-arch: + type: string + default: "" docker: - image: *GOLANG_IMAGE environment: GOTAGS: "" # No tags for OSS but there are for enterprise + GOARCH: "<>" steps: - checkout + - run: go env - run: name: Install golangci-lint command: | @@ -275,6 +281,40 @@ jobs: path: /tmp/jsonfile - run: *notify-slack-failure + # go-test-32bit is to catch problems where 64-bit ints must be 64-bit aligned + # to use them with sync/atomic. See https://golang.org/pkg/sync/atomic/#pkg-note-BUG. + # Running tests with GOARCH=386 seems to be the best way to detect this + # problem. Only runs tests that are -short to limit the time we spend checking + # for these bugs. + go-test-32bit: + docker: + - image: *GOLANG_IMAGE + environment: + <<: *ENVIRONMENT + GOTAGS: "" # No tags for OSS but there are for enterprise + steps: + - checkout + - run: *install-gotestsum + - run: go mod download + - run: + name: go test 32-bit + environment: + GOARCH: 386 + command: | + mkdir -p $TEST_RESULTS_DIR /tmp/jsonfile + go env + gotestsum \ + --jsonfile /tmp/jsonfile/go-test-32bit.log \ + --junitfile $TEST_RESULTS_DIR/gotestsum-report.xml -- \ + -tags="$GOTAGS" -p 2 \ + -short ./... + + - store_test_results: + path: *TEST_RESULTS_DIR + - store_artifacts: + path: *TEST_RESULTS_DIR + - run: *notify-slack-failure + go-test-lib: description: "test a library against a specific Go version" parameters: @@ -961,6 +1001,10 @@ workflows: - check-generated-protobuf: *filter-ignore-non-go-branches - lint-consul-retry: *filter-ignore-non-go-branches - lint: *filter-ignore-non-go-branches + - lint: + name: "lint-32bit" + go-arch: "386" + <<: *filter-ignore-non-go-branches - test-connect-ca-providers: *filter-ignore-non-go-branches - dev-build: *filter-ignore-non-go-branches - go-test: @@ -986,6 +1030,7 @@ workflows: go-version: "1.15" <<: *filter-ignore-non-go-branches - go-test-race: *filter-ignore-non-go-branches + - go-test-32bit: *filter-ignore-non-go-branches build-distros: unless: << pipeline.parameters.trigger-load-test >> jobs: diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index c963e78ba8..0f02aadea2 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -5473,7 +5473,7 @@ func TestLoad_FullConfig(t *testing.T) { HTTPSPort: 15127, HTTPUseCache: false, KeyFile: "IEkkwgIA", - KVMaxValueSize: 1234567800000000, + KVMaxValueSize: 1234567800, LeaveDrainTime: 8265 * time.Second, LeaveOnTerm: true, Logging: logging.Config{ @@ -5868,7 +5868,7 @@ func TestLoad_FullConfig(t *testing.T) { "wan_ipv4": "78.63.37.19", }, TranslateWANAddrs: true, - TxnMaxReqLen: 5678000000000000, + TxnMaxReqLen: 567800000, UIConfig: UIConfig{ Dir: "pVncV4Ey", ContentPath: "/qp1WRhYH/", // slashes are added in parsing diff --git a/agent/config/testdata/full-config.hcl b/agent/config/testdata/full-config.hcl index 0cc708a701..6cf06ff232 100644 --- a/agent/config/testdata/full-config.hcl +++ b/agent/config/testdata/full-config.hcl @@ -285,8 +285,8 @@ limits { rpc_rate = 12029.43 rpc_max_burst = 44848 rpc_max_conns_per_client = 2954 - kv_max_value_size = 1234567800000000 - txn_max_req_len = 5678000000000000 + kv_max_value_size = 1234567800 + txn_max_req_len = 567800000 } log_level = "k1zo9Spt" log_json = true diff --git a/agent/config/testdata/full-config.json b/agent/config/testdata/full-config.json index adb3a7cfa9..2dd76c6f1a 100644 --- a/agent/config/testdata/full-config.json +++ b/agent/config/testdata/full-config.json @@ -285,8 +285,8 @@ "rpc_rate": 12029.43, "rpc_max_burst": 44848, "rpc_max_conns_per_client": 2954, - "kv_max_value_size": 1234567800000000, - "txn_max_req_len": 5678000000000000 + "kv_max_value_size": 1234567800, + "txn_max_req_len": 567800000 }, "log_level": "k1zo9Spt", "log_json": true, diff --git a/agent/grpc/stats.go b/agent/grpc/stats.go index 7ba96f91f4..8af6ec4005 100644 --- a/agent/grpc/stats.go +++ b/agent/grpc/stats.go @@ -52,8 +52,11 @@ var defaultMetrics = metrics.Default // statsHandler is a grpc/stats.StatsHandler which emits connection and // request metrics to go-metrics. type statsHandler struct { + // activeConns is used with sync/atomic and MUST be 64-bit aligned. To ensure + // alignment on 32-bit platforms this field must remain the first field in + // the struct. See https://golang.org/pkg/sync/atomic/#pkg-note-BUG. + activeConns uint64 metrics *metrics.Metrics - activeConns uint64 // must be 8-byte aligned for atomic access } func newStatsHandler(m *metrics.Metrics) *statsHandler { @@ -103,10 +106,11 @@ func (c *statsHandler) HandleConn(_ context.Context, s stats.ConnStats) { } type activeStreamCounter struct { + // count is used with sync/atomic and MUST be 64-bit aligned. To ensure + // alignment on 32-bit platforms this field must remain the first field in + // the struct. See https://golang.org/pkg/sync/atomic/#pkg-note-BUG. + count uint64 metrics *metrics.Metrics - // count of the number of open streaming RPCs on a server. It is accessed - // atomically. - count uint64 } // GRPCCountingStreamInterceptor is a grpc.ServerStreamInterceptor that emits a diff --git a/command/connect/redirecttraffic/redirect_traffic_test.go b/command/connect/redirecttraffic/redirect_traffic_test.go index 0fde8a37ec..5c31af036c 100644 --- a/command/connect/redirecttraffic/redirect_traffic_test.go +++ b/command/connect/redirecttraffic/redirect_traffic_test.go @@ -59,6 +59,10 @@ func TestRun_FlagValidation(t *testing.T) { } func TestGenerateConfigFromFlags(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + cases := []struct { name string command func() cmd