Merge pull request #73 from guggero/race-fix

unit-race: fix race condition in unit tests

.github/workflows/main.yml

@@ -0,0 +1,94 @@
+name: CI
+
+on:
+  push:
+    branches:
+      - "master"
+  pull_request:
+    branches:
+      - "*"
+
+defaults:
+  run:
+    shell: bash
+
+env:
+  # go needs absolute directories, using the $HOME variable doesn't work here.
+  GOCACHE: /home/runner/work/go/pkg/build
+  GOPATH: /home/runner/work/go
+
+  # If you change this value, please change it in the following files as well:
+  # /Dockerfile
+  #
+  # Don't bump this until go 1.19 is out (which should include a fix for
+  # https://github.com/golang/go/issues/51799). There was a race condition
+  # introduced with go 1.16.10 that causes the unit tests to fail (could also
+  # happen in production).
+  GO_VERSION: 1.16.9
+
+jobs:
+  ########################
+  # lint code
+  ########################
+  lint:
+    name: lint code
+    runs-on: ubuntu-latest
+    steps:
+      - name: git checkout
+        uses: actions/checkout@v2
+        with:
+          fetch-depth: 0
+
+      - name: go cache
+        uses: actions/cache@v1
+        with:
+          path: /home/runner/work/go
+          key: subasta-${{ runner.os }}-go-${{ env.GO_VERSION }}-${{ github.job }}-${{ hashFiles('**/go.sum') }}
+          restore-keys: |
+            aperture-${{ runner.os }}-go-${{ env.GO_VERSION }}-${{ github.job }}-${{ hashFiles('**/go.sum') }}
+            aperture-${{ runner.os }}-go-${{ env.GO_VERSION }}-${{ github.job }}-
+            aperture-${{ runner.os }}-go-${{ env.GO_VERSION }}-
+            aperture-${{ runner.os }}-go-
+
+      - name: setup go ${{ env.GO_VERSION }}
+        uses: actions/setup-go@v2
+        with:
+          go-version: '${{ env.GO_VERSION }}'
+
+      - name: lint
+        run: make lint
+
+  ########################
+  # run unit tests
+  ########################
+  unit-test:
+    name: run unit tests
+    runs-on: ubuntu-latest
+    strategy:
+      # Allow other tests in the matrix to continue if one fails.
+      fail-fast: false
+      matrix:
+        unit_type:
+          - unit-race
+    steps:
+      - name: git checkout
+        uses: actions/checkout@v2
+
+      - name: go cache
+        uses: actions/cache@v1
+        with:
+          path: /home/runner/work/go
+          key: subasta-${{ runner.os }}-go-${{ env.GO_VERSION }}-${{ github.job }}-${{ hashFiles('**/go.sum') }}
+          restore-keys: |
+            aperture-${{ runner.os }}-go-${{ env.GO_VERSION }}-${{ github.job }}-${{ hashFiles('**/go.sum') }}
+            aperture-${{ runner.os }}-go-${{ env.GO_VERSION }}-${{ github.job }}-
+            aperture-${{ runner.os }}-go-${{ env.GO_VERSION }}-
+            aperture-${{ runner.os }}-go-
+
+      - name: setup go ${{ env.GO_VERSION }}
+        uses: actions/setup-go@v2
+        with:
+          go-version: '${{ env.GO_VERSION }}'
+
+      - name: run ${{ matrix.unit_type }}
+        run: make ${{ matrix.unit_type }}

.travis.yml

@@ -1,16 +0,0 @@
-language: go
-
-go:
-  - "1.16.x"
-
-cache:
-  directories:
-  - $GOCACHE
-  - $GOPATH/pkg/mod
-
-env:
-  global:
-    - GOCACHE=$HOME/.go-build
-
-script:
-  - make travis-race

Dockerfile

@@ -1,4 +1,8 @@
-FROM golang:1.15.5-alpine as builder
+# Don't bump this until go 1.19 is out (which should include a fix for
+# https://github.com/golang/go/issues/51799). There was a race condition
+# introduced with go 1.16.10 that causes the unit tests to fail (could also
+# happen in production).
+FROM golang:1.16.9-alpine as builder
 
 # Force Go to use the cgo based DNS resolver. This is required to ensure DNS
 # queries required to connect to linked containers succeed.

aperture.go

@@ -680,22 +680,27 @@ func createProxy(cfg *Config, challenger *LndChallenger,
 func createHashMailServer(cfg *Config) ([]proxy.LocalService, func(), error) {
 	var localServices []proxy.LocalService
 
-	// Before we register both servers, we'll also ensure that the
-	// collector will export latency metrics for the histogram.
-	grpc_prometheus.EnableHandlingTimeHistogram()
-
 	serverOpts := []grpc.ServerOption{
-		grpc.ChainUnaryInterceptor(
-			grpc_prometheus.UnaryServerInterceptor,
-		),
-		grpc.ChainStreamInterceptor(
-			grpc_prometheus.StreamServerInterceptor,
-		),
 		grpc.KeepaliveEnforcementPolicy(keepalive.EnforcementPolicy{
 			MinTime: time.Minute,
 		}),
 	}
 
+	// Before we register both servers, we'll also ensure that the collector
+	// will export latency metrics for the histogram.
+	if cfg.Prometheus != nil && cfg.Prometheus.Enabled {
+		grpc_prometheus.EnableHandlingTimeHistogram()
+		serverOpts = append(
+			serverOpts,
+			grpc.ChainUnaryInterceptor(
+				grpc_prometheus.UnaryServerInterceptor,
+			),
+			grpc.ChainStreamInterceptor(
+				grpc_prometheus.StreamServerInterceptor,
+			),
+		)
+	}
+
 	// Create a gRPC server for the hashmail server.
 	hashMailServer := newHashMailServer(hashMailServerConfig{
 		msgRate:           cfg.HashMail.MessageRate,
@@ -710,7 +715,9 @@ func createHashMailServer(cfg *Config) ([]proxy.LocalService, func(), error) {
 	)
 
 	// Export the gRPC information for the public gRPC server.
-	grpc_prometheus.Register(hashMailGRPC)
+	if cfg.Prometheus != nil && cfg.Prometheus.Enabled {
+		grpc_prometheus.Register(hashMailGRPC)
+	}
 
 	// And a REST proxy for it as well.
 	// The default JSON marshaler of the REST proxy only sets OrigName to