fix: multiple data race issues

See siderolabs/talos#9466 There were multiple issues: * a pointer to a slice element might point to a different element if the slice contents change (chunks are recycled) * compression buffer can't be reused, as it might be still used in the reader doing decompression * incorrect handling of growing initial buffer in streaming reads Signed-off-by: Andrey Smirnov <andrey.smirnov@siderolabs.com>
siderolabs · Oct 15, 2024 · 9a0f7b0 · 9a0f7b0
1 parent cbce5c3
commit 9a0f7b0
Show file tree

Hide file tree

Showing 10 changed files with 281 additions and 114 deletions.
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -1,6 +1,6 @@
 # THIS FILE WAS AUTOMATICALLY GENERATED, PLEASE DO NOT EDIT.
 #
-# Generated on 2024-05-08T11:26:09Z by kres 1e986af.
+# Generated on 2024-10-15T16:00:31Z by kres 34e72ac.
 
 name: default
 concurrency:
@@ -29,16 +29,32 @@ jobs:
       - self-hosted
       - generic
     if: (!startsWith(github.head_ref, 'renovate/') && !startsWith(github.head_ref, 'dependabot/'))
-    services:
-      buildkitd:
-        image: moby/buildkit:v0.13.2
-        options: --privileged
-        ports:
-          - 1234:1234
-        volumes:
-          - /var/lib/buildkit/${{ github.repository }}:/var/lib/buildkit
-          - /usr/etc/buildkit/buildkitd.toml:/etc/buildkit/buildkitd.toml
     steps:
+      - name: gather-system-info
+        id: system-info
+        uses: kenchan0130/actions-system-info@v1.3.0
+        continue-on-error: true
+      - name: print-system-info
+        run: |
+          MEMORY_GB=$((${{ steps.system-info.outputs.totalmem }}/1024/1024/1024))
+
+          OUTPUTS=(
+            "CPU Core: ${{ steps.system-info.outputs.cpu-core }}"
+            "CPU Model: ${{ steps.system-info.outputs.cpu-model }}"
+            "Hostname: ${{ steps.system-info.outputs.hostname }}"
+            "NodeName: ${NODE_NAME}"
+            "Kernel release: ${{ steps.system-info.outputs.kernel-release }}"
+            "Kernel version: ${{ steps.system-info.outputs.kernel-version }}"
+            "Name: ${{ steps.system-info.outputs.name }}"
+            "Platform: ${{ steps.system-info.outputs.platform }}"
+            "Release: ${{ steps.system-info.outputs.release }}"
+            "Total memory: ${MEMORY_GB} GB"
+          )
+
+          for OUTPUT in "${OUTPUTS[@]}";do
+            echo "${OUTPUT}"
+          done
+        continue-on-error: true
       - name: checkout
         uses: actions/checkout@v4
       - name: Unshallow
@@ -49,7 +65,7 @@ jobs:
         uses: docker/setup-buildx-action@v3
         with:
           driver: remote
-          endpoint: tcp://127.0.0.1:1234
+          endpoint: tcp://buildkit-amd64.ci.svc.cluster.local:1234
         timeout-minutes: 10
       - name: base
         run: |

diff --git a/.golangci.yml b/.golangci.yml
@@ -1,6 +1,6 @@
 # THIS FILE WAS AUTOMATICALLY GENERATED, PLEASE DO NOT EDIT.
 #
-# Generated on 2024-05-16T11:12:28Z by kres ce88e1c.
+# Generated on 2024-10-15T16:00:31Z by kres 34e72ac.
 
 # options for analysis running
 run:
@@ -13,8 +13,8 @@ run:
 # output configuration options
 output:
   formats:
-  - format: colored-line-number
-    path: stdout
+    - format: colored-line-number
+      path: stdout
   print-issued-lines: true
   print-linter-name: true
   uniq-by-line: true
@@ -35,7 +35,7 @@ linters-settings:
     sections:
       - standard # Standard section: captures all standard packages.
       - default # Default section: contains all imports that could not be matched to another section type.
-      - localmodule
+      - localmodule # Imports from the same module.
   gocognit:
     min-complexity: 30
   nestif:
@@ -51,8 +51,6 @@ linters-settings:
     scope: declarations
   gofmt:
     simplify: true
-  goimports:
-    local-prefixes: github.com/siderolabs/go-circular/
   gomodguard: { }
   govet:
     enable-all: true
@@ -96,17 +94,21 @@ linters-settings:
   cyclop:
     # the maximal code complexity to report
     max-complexity: 20
-  #  depguard:
-  #    Main:
-  #      deny:
-  #        - github.com/OpenPeeDeeP/depguard # this is just an example
+  depguard:
+    rules:
+      prevent_unmaintained_packages:
+        list-mode: lax # allow unless explicitly denied
+        files:
+          - $all
+        deny:
+          - pkg: io/ioutil
+            desc: "replaced by io and os packages since Go 1.16: https://tip.golang.org/doc/go1.16#ioutil"
 
 linters:
   enable-all: true
   disable-all: false
   fast: false
   disable:
-    - exhaustivestruct
     - exhaustruct
     - err113
     - forbidigo
@@ -122,29 +124,17 @@ linters:
     - mnd
     - nestif
     - nonamedreturns
-    - nosnakecase
     - paralleltest
     - tagalign
     - tagliatelle
     - thelper
-    - typecheck
     - varnamelen
     - wrapcheck
-    - depguard # Disabled because starting with golangci-lint 1.53.0 it doesn't allow denylist alone anymore
     - testifylint # complains about our assert recorder and has a number of false positives for assert.Greater(t, thing, 1)
     - protogetter # complains about us using Value field on typed spec, instead of GetValue which has a different signature
     - perfsprint # complains about us using fmt.Sprintf in non-performance critical code, updating just kres took too long
-    # abandoned linters for which golangci shows the warning that the repo is archived by the owner
-    - deadcode
-    - golint
-    - ifshort
-    - interfacer
-    - maligned
-    - scopelint
-    - structcheck
-    - varcheck
-    # disabled as it seems to be broken - goes into imported libraries and reports issues there
-    - musttag
+    - goimports # same as gci
+    - musttag # seems to be broken - goes into imported libraries and reports issues there
 
 issues:
   exclude: [ ]

diff --git a/Dockerfile b/Dockerfile
@@ -1,37 +1,36 @@
-# syntax = docker/dockerfile-upstream:1.7.1-labs
+# syntax = docker/dockerfile-upstream:1.10.0-labs
 
 # THIS FILE WAS AUTOMATICALLY GENERATED, PLEASE DO NOT EDIT.
 #
-# Generated on 2024-05-16T11:12:28Z by kres ce88e1c.
+# Generated on 2024-10-15T16:00:31Z by kres 34e72ac.
 
 ARG TOOLCHAIN
 
 # cleaned up specs and compiled versions
 FROM scratch AS generate
 
 # runs markdownlint
-FROM docker.io/node:21.7.3-alpine3.19 AS lint-markdown
+FROM docker.io/oven/bun:1.1.29-alpine AS lint-markdown
 WORKDIR /src
-RUN npm i -g markdownlint-cli@0.39.0
-RUN npm i sentences-per-line@0.2.1
+RUN bun i markdownlint-cli@0.41.0 sentences-per-line@0.2.1
 COPY .markdownlint.json .
 COPY ./README.md ./README.md
-RUN markdownlint --ignore "CHANGELOG.md" --ignore "**/node_modules/**" --ignore '**/hack/chglog/**' --rules node_modules/sentences-per-line/index.js .
+RUN bunx markdownlint --ignore "CHANGELOG.md" --ignore "**/node_modules/**" --ignore '**/hack/chglog/**' --rules node_modules/sentences-per-line/index.js .
 
 # base toolchain image
-FROM ${TOOLCHAIN} AS toolchain
+FROM --platform=${BUILDPLATFORM} ${TOOLCHAIN} AS toolchain
 RUN apk --update --no-cache add bash curl build-base protoc protobuf-dev
 
 # build tools
 FROM --platform=${BUILDPLATFORM} toolchain AS tools
-ENV GO111MODULE on
+ENV GO111MODULE=on
 ARG CGO_ENABLED
-ENV CGO_ENABLED ${CGO_ENABLED}
+ENV CGO_ENABLED=${CGO_ENABLED}
 ARG GOTOOLCHAIN
-ENV GOTOOLCHAIN ${GOTOOLCHAIN}
+ENV GOTOOLCHAIN=${GOTOOLCHAIN}
 ARG GOEXPERIMENT
-ENV GOEXPERIMENT ${GOEXPERIMENT}
-ENV GOPATH /go
+ENV GOEXPERIMENT=${GOEXPERIMENT}
+ENV GOPATH=/go
 ARG DEEPCOPY_VERSION
 RUN --mount=type=cache,target=/root/.cache/go-build --mount=type=cache,target=/go/pkg go install github.com/siderolabs/deep-copy@${DEEPCOPY_VERSION} \
 	&& mv /go/bin/deep-copy /bin/deep-copy
@@ -40,9 +39,6 @@ RUN --mount=type=cache,target=/root/.cache/go-build --mount=type=cache,target=/g
 	&& mv /go/bin/golangci-lint /bin/golangci-lint
 RUN --mount=type=cache,target=/root/.cache/go-build --mount=type=cache,target=/go/pkg go install golang.org/x/vuln/cmd/govulncheck@latest \
 	&& mv /go/bin/govulncheck /bin/govulncheck
-ARG GOIMPORTS_VERSION
-RUN --mount=type=cache,target=/root/.cache/go-build --mount=type=cache,target=/go/pkg go install golang.org/x/tools/cmd/goimports@${GOIMPORTS_VERSION} \
-	&& mv /go/bin/goimports /bin/goimports
 ARG GOFUMPT_VERSION
 RUN go install mvdan.cc/gofumpt@${GOFUMPT_VERSION} \
 	&& mv /go/bin/gofumpt /bin/gofumpt
@@ -72,15 +68,11 @@ RUN --mount=type=cache,target=/go/pkg go list -mod=readonly all >/dev/null
 FROM base AS lint-gofumpt
 RUN FILES="$(gofumpt -l .)" && test -z "${FILES}" || (echo -e "Source code is not formatted with 'gofumpt -w .':\n${FILES}"; exit 1)
 
-# runs goimports
-FROM base AS lint-goimports
-RUN FILES="$(goimports -l -local github.com/siderolabs/go-circular/ .)" && test -z "${FILES}" || (echo -e "Source code is not formatted with 'goimports -w -local github.com/siderolabs/go-circular/ .':\n${FILES}"; exit 1)
-
 # runs golangci-lint
 FROM base AS lint-golangci-lint
 WORKDIR /src
 COPY .golangci.yml .
-ENV GOGC 50
+ENV GOGC=50
 RUN golangci-lint config verify --config .golangci.yml
 RUN --mount=type=cache,target=/root/.cache/go-build --mount=type=cache,target=/root/.cache/golangci-lint --mount=type=cache,target=/go/pkg golangci-lint run --config .golangci.yml
 

diff --git a/Makefile b/Makefile
@@ -1,6 +1,6 @@
 # THIS FILE WAS AUTOMATICALLY GENERATED, PLEASE DO NOT EDIT.
 #
-# Generated on 2024-05-08T11:26:09Z by kres 1e986af.
+# Generated on 2024-10-15T16:00:31Z by kres 34e72ac.
 
 # common variables
 
@@ -10,20 +10,22 @@ ABBREV_TAG := $(shell git describe --tags >/dev/null 2>/dev/null && git describe
 BRANCH := $(shell git rev-parse --abbrev-ref HEAD)
 ARTIFACTS := _out
 IMAGE_TAG ?= $(TAG)
+OPERATING_SYSTEM := $(shell uname -s | tr '[:upper:]' '[:lower:]')
+GOARCH := $(shell uname -m | sed 's/x86_64/amd64/' | sed 's/aarch64/arm64/')
 WITH_DEBUG ?= false
 WITH_RACE ?= false
 REGISTRY ?= ghcr.io
 USERNAME ?= siderolabs
 REGISTRY_AND_USERNAME ?= $(REGISTRY)/$(USERNAME)
-PROTOBUF_GO_VERSION ?= 1.33.0
-GRPC_GO_VERSION ?= 1.3.0
-GRPC_GATEWAY_VERSION ?= 2.19.1
+PROTOBUF_GO_VERSION ?= 1.34.2
+GRPC_GO_VERSION ?= 1.5.1
+GRPC_GATEWAY_VERSION ?= 2.22.0
 VTPROTOBUF_VERSION ?= 0.6.0
+GOIMPORTS_VERSION ?= 0.25.0
 DEEPCOPY_VERSION ?= v0.5.6
-GOLANGCILINT_VERSION ?= v1.58.0
-GOFUMPT_VERSION ?= v0.6.0
-GO_VERSION ?= 1.22.3
-GOIMPORTS_VERSION ?= v0.20.0
+GOLANGCILINT_VERSION ?= v1.61.0
+GOFUMPT_VERSION ?= v0.7.0
+GO_VERSION ?= 1.23.2
 GO_BUILDFLAGS ?=
 GO_LDFLAGS ?=
 CGO_ENABLED ?= 0
@@ -60,12 +62,12 @@ COMMON_ARGS += --build-arg=PROTOBUF_GO_VERSION="$(PROTOBUF_GO_VERSION)"
 COMMON_ARGS += --build-arg=GRPC_GO_VERSION="$(GRPC_GO_VERSION)"
 COMMON_ARGS += --build-arg=GRPC_GATEWAY_VERSION="$(GRPC_GATEWAY_VERSION)"
 COMMON_ARGS += --build-arg=VTPROTOBUF_VERSION="$(VTPROTOBUF_VERSION)"
+COMMON_ARGS += --build-arg=GOIMPORTS_VERSION="$(GOIMPORTS_VERSION)"
 COMMON_ARGS += --build-arg=DEEPCOPY_VERSION="$(DEEPCOPY_VERSION)"
 COMMON_ARGS += --build-arg=GOLANGCILINT_VERSION="$(GOLANGCILINT_VERSION)"
-COMMON_ARGS += --build-arg=GOIMPORTS_VERSION="$(GOIMPORTS_VERSION)"
 COMMON_ARGS += --build-arg=GOFUMPT_VERSION="$(GOFUMPT_VERSION)"
 COMMON_ARGS += --build-arg=TESTPKGS="$(TESTPKGS)"
-TOOLCHAIN ?= docker.io/golang:1.22-alpine
+TOOLCHAIN ?= docker.io/golang:1.23-alpine
 
 # help menu
 
@@ -131,6 +133,9 @@ endif
 
 all: unit-tests lint
 
+$(ARTIFACTS):  ## Creates artifacts directory.
+	@mkdir -p $(ARTIFACTS)
+
 .PHONY: clean
 clean:  ## Cleans up all artifacts.
 	@rm -rf $(ARTIFACTS)
@@ -158,9 +163,6 @@ fmt:  ## Formats the source code
 lint-govulncheck:  ## Runs govulncheck linter.
 	@$(MAKE) target-$@
 
-lint-goimports:  ## Runs goimports linter.
-	@$(MAKE) target-$@
-
 .PHONY: base
 base:  ## Prepare base toolchain
 	@$(MAKE) target-$@
@@ -178,7 +180,7 @@ lint-markdown:  ## Runs markdownlint.
 	@$(MAKE) target-$@
 
 .PHONY: lint
-lint: lint-golangci-lint lint-gofumpt lint-govulncheck lint-goimports lint-markdown  ## Run all linters for the project.
+lint: lint-golangci-lint lint-gofumpt lint-govulncheck lint-markdown  ## Run all linters for the project.
 
 .PHONY: rekres
 rekres:
@@ -191,8 +193,7 @@ help:  ## This help menu.
 	@grep -E '^[a-zA-Z%_-]+:.*?## .*$$' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'
 
 .PHONY: release-notes
-release-notes:
-	mkdir -p $(ARTIFACTS)
+release-notes: $(ARTIFACTS)
 	@ARTIFACTS=$(ARTIFACTS) ./hack/release.sh $@ $(ARTIFACTS)/RELEASE_NOTES.md $(TAG)
 
 .PHONY: conformance

diff --git a/circular.go b/circular.go
@@ -11,6 +11,8 @@ import (
 	"slices"
 	"sync"
 	"sync/atomic"
+
+	"github.com/siderolabs/gen/optional"
 )
 
 // Buffer implements circular buffer with a thread-safe writer,
@@ -147,14 +149,8 @@ func (buf *Buffer) Write(p []byte) (int, error) {
 		p = p[nn:]
 
 		if rotate && buf.opt.NumCompressedChunks > 0 {
-			var compressionBuf []byte
-
-			if len(buf.chunks) == buf.opt.NumCompressedChunks {
-				// going to drop the chunk, so reuse its buffer
-				compressionBuf = buf.chunks[0].compressed[:0]
-			}
-
-			compressed, err := buf.opt.Compressor.Compress(buf.data, compressionBuf)
+			// we can't reuse any of the chunk buffers, as they might be referenced by readers
+			compressed, err := buf.opt.Compressor.Compress(buf.data, nil)
 			if err != nil {
 				return n, err
 			}
@@ -285,7 +281,7 @@ func (buf *Buffer) GetReader() *Reader {
 		return &Reader{
 			buf: buf,
 
-			chunk: &oldestChunk,
+			chunk: optional.Some(oldestChunk),
 
 			startOff: oldestChunk.startOffset,
 			endOff:   buf.off,