Skip to content

Commit cc29a9f

Browse files
bgentrybgentry
authored
close shared pool before goleak check (#1169)
CI intermittently failed in `test/race` with a `goleak` report for `github.com/jackc/pgx/v5/pgconn/internal/bgreader.(*BGReader).bgRead`: goleak: Errors on successful test run: found unexpected goroutines: [Goroutine ... with github.com/jackc/pgx/v5/pgconn/internal/bgreader.(*BGReader).bgRead on top of the stack ...] created by github.com/jackc/pgx/v5/pgconn/internal/bgreader.(*BGReader).Start Failing run: https://github.com/riverqueue/river/actions/runs/23095300730/job/67086685003 That stack does not point to a River-managed goroutine. It means a pgx connection was still open when leak checking ran. In our test helpers, the shared Postgres pool was intentionally kept alive until process exit, but `goleak.Find` runs before exit. That allowed idle pgx background reader goroutines to still be blocked in socket reads during teardown even when package tests had otherwise finished cleanly. Fix this by explicitly closing the shared test pool before running `goleak.Find` in the shared `TestMain` wrapper. Internal tests now reuse that shared wrapper directly so there is only one leak-checking path to maintain. This addresses the flake at its source and keeps `goleak` useful, rather than globally ignoring all `bgRead` goroutines and potentially hiding real connection leaks.
1 parent 1eb91bc commit cc29a9f

17 files changed

Lines changed: 139 additions & 61 deletions

File tree

‎.github/workflows/ci.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ jobs:
299299
name: lint
300300
runs-on: ubuntu-latest
301301
env:
302-
GOLANGCI_LINT_VERSION: v2.9.0
302+
GOLANGCI_LINT_VERSION: v2.10.1
303303
permissions:
304304
contents: read
305305
# allow read access to pull request. Use with `only-new-issues` option.

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ require (
1616
github.com/stretchr/testify v1.11.1
1717
github.com/tidwall/gjson v1.18.0
1818
github.com/tidwall/sjson v1.2.5
19-
go.uber.org/goleak v1.3.0
2019
golang.org/x/sync v0.20.0
2120
)
2221

@@ -27,6 +26,7 @@ require (
2726
github.com/pmezard/go-difflib v1.0.0 // indirect
2827
github.com/tidwall/match v1.2.0 // indirect
2928
github.com/tidwall/pretty v1.2.1 // indirect
29+
go.uber.org/goleak v1.3.0 // indirect
3030
golang.org/x/text v0.34.0 // indirect
3131
gopkg.in/yaml.v3 v3.0.1 // indirect
3232
)

‎internal/jobexecutor/job_executor.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,7 @@ func captureStackTraceSkipFrames(skip int) string {
531531
var stackTraceSB strings.Builder
532532
for {
533533
frame, more := frames.Next()
534-
stackTraceSB.WriteString(fmt.Sprintf("%s\n\t%s:%d\n", frame.Function, frame.File, frame.Line))
534+
fmt.Fprintf(&stackTraceSB, "%s\n\t%s:%d\n", frame.Function, frame.File, frame.Line)
535535
if !more {
536536
break
537537
}

‎internal/riverinternaltest/riverinternaltest.go‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,10 @@
33
package riverinternaltest
44

55
import (
6-
"fmt"
7-
"os"
86
"sync"
97
"testing"
108
"time"
119

12-
"go.uber.org/goleak"
13-
1410
"github.com/riverqueue/river/rivershared/riversharedtest"
1511
)
1612

@@ -103,14 +99,5 @@ func DrainContinuously[T any](drainChan <-chan T) func() []T {
10399
// WrapTestMain performs some common setup and teardown that should be shared
104100
// amongst all packages. e.g. Checks for no goroutine leaks on teardown.
105101
func WrapTestMain(m *testing.M) {
106-
status := m.Run()
107-
108-
if status == 0 {
109-
if err := goleak.Find(riversharedtest.IgnoredKnownGoroutineLeaks...); err != nil {
110-
fmt.Fprintf(os.Stderr, "goleak: Errors on successful test run: %v\n", err)
111-
status = 1
112-
}
113-
}
114-
115-
os.Exit(status)
102+
riversharedtest.WrapTestMain(m)
116103
}

‎riverdriver/go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
77
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
88
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
99
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
10-
github.com/riverqueue/river/rivertype v0.29.0-rc.1 h1:FSHV9EUugtOY3nhly9LfxUIOIrS1Sv0TkbKvWQO6940=
11-
github.com/riverqueue/river/rivertype v0.29.0-rc.1/go.mod h1:rWpgI59doOWS6zlVocROcwc00fZ1RbzRwsRTU8CDguw=
10+
github.com/riverqueue/river/rivertype v0.31.0 h1:O6vaJ72SffgF1nxzCrDKd4M+eMZFRlJpycnOcUIGLD8=
11+
github.com/riverqueue/river/rivertype v0.31.0/go.mod h1:D1Ad+EaZiaXbQbJcJcfeicXJMBKno0n6UcfKI5Q7DIQ=
1212
github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8=
1313
github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4=
1414
github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=

‎riverdriver/riverdatabasesql/river_database_sql_driver.go‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -629,8 +629,12 @@ func (e *Executor) JobSetStateIfRunningMany(ctx context.Context, params *riverdr
629629
for i := range len(params.ID) {
630630
setStateParams.Errors[i] = cmp.Or(string(params.ErrData[i]), defaultObject)
631631
if params.Attempt[i] != nil {
632+
attempt, err := intToInt32(*params.Attempt[i])
633+
if err != nil {
634+
return nil, err
635+
}
632636
setStateParams.AttemptDoUpdate[i] = true
633-
setStateParams.Attempt[i] = int32(*params.Attempt[i]) //nolint:gosec
637+
setStateParams.Attempt[i] = attempt
634638
}
635639
if params.ErrData[i] != nil {
636640
setStateParams.ErrorsDoUpdate[i] = true
@@ -1140,6 +1144,22 @@ func bitIntegerToBits(bitInteger, numBits int) int {
11401144
return bits
11411145
}
11421146

1147+
func intToByte(value int) (byte, error) {
1148+
if value < 0 || value > 255 {
1149+
return 0, fmt.Errorf("value out of range for byte: %d", value)
1150+
}
1151+
1152+
return byte(value), nil
1153+
}
1154+
1155+
func intToInt32(value int) (int32, error) {
1156+
if value < math.MinInt32 || value > math.MaxInt32 {
1157+
return 0, fmt.Errorf("value out of range for int32: %d", value)
1158+
}
1159+
1160+
return int32(value), nil
1161+
}
1162+
11431163
func jobRowFromInternal(internal *dbsqlc.RiverJob) (*rivertype.JobRow, error) {
11441164
var attemptedAt *time.Time
11451165
if internal.AttemptedAt != nil {
@@ -1160,6 +1180,11 @@ func jobRowFromInternal(internal *dbsqlc.RiverJob) (*rivertype.JobRow, error) {
11601180
finalizedAt = &t
11611181
}
11621182

1183+
uniqueStates, err := intToByte(bitIntegerToBits(ptrutil.ValOrDefault(internal.UniqueStates, 0), 8))
1184+
if err != nil {
1185+
return nil, err
1186+
}
1187+
11631188
return &rivertype.JobRow{
11641189
ID: internal.ID,
11651190
Attempt: max(int(internal.Attempt), 0),
@@ -1178,7 +1203,7 @@ func jobRowFromInternal(internal *dbsqlc.RiverJob) (*rivertype.JobRow, error) {
11781203
State: rivertype.JobState(internal.State),
11791204
Tags: internal.Tags,
11801205
UniqueKey: internal.UniqueKey,
1181-
UniqueStates: uniquestates.UniqueBitmaskToStates(byte(bitIntegerToBits(ptrutil.ValOrDefault(internal.UniqueStates, 0), 8))),
1206+
UniqueStates: uniquestates.UniqueBitmaskToStates(uniqueStates),
11821207
}, nil
11831208
}
11841209

‎riverdriver/riverpgxv5/river_pgx_v5_driver.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -623,7 +623,7 @@ func (e *Executor) JobSetStateIfRunningMany(ctx context.Context, params *riverdr
623623
for i := range len(params.ID) {
624624
if params.Attempt[i] != nil {
625625
setStateParams.AttemptDoUpdate[i] = true
626-
setStateParams.Attempt[i] = int32(*params.Attempt[i]) //nolint:gosec
626+
setStateParams.Attempt[i] = int32(*params.Attempt[i])
627627
}
628628
if params.ErrData[i] != nil {
629629
setStateParams.ErrorsDoUpdate[i] = true

‎riverdriver/riversqlite/go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ require (
1717
require (
1818
github.com/davecgh/go-spew v1.1.1 // indirect
1919
github.com/pmezard/go-difflib v1.0.0 // indirect
20-
github.com/tidwall/match v1.1.1 // indirect
20+
github.com/tidwall/match v1.2.0 // indirect
2121
github.com/tidwall/pretty v1.2.1 // indirect
2222
gopkg.in/yaml.v3 v3.0.1 // indirect
2323
)

‎riverdriver/riversqlite/go.sum‎

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ github.com/jackc/pgpassfile v1.0.0 h1:/6Hmqy13Ss2zCq62VdNG8tM1wchn8zjSGOBJ6icpsI
44
github.com/jackc/pgpassfile v1.0.0/go.mod h1:CEx0iS5ambNFdcRtxPj5JhEz+xB6uRky5eyVu/W2HEg=
55
github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 h1:iCEnooe7UlwOQYpKFhBabPMi4aNAfoODPEFNiAnClxo=
66
github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761/go.mod h1:5TJZWKEWniPve33vlWYSoGYefn3gLQRzjfDlhSJ9ZKM=
7-
github.com/jackc/pgx/v5 v5.7.6 h1:rWQc5FwZSPX58r1OQmkuaNicxdmExaEz5A2DO2hUuTk=
8-
github.com/jackc/pgx/v5 v5.7.6/go.mod h1:aruU7o91Tc2q2cFp5h4uP3f6ztExVpyVv88Xl/8Vl8M=
7+
github.com/jackc/pgx/v5 v5.8.0 h1:TYPDoleBBme0xGSAX3/+NujXXtpZn9HBONkQC7IEZSo=
8+
github.com/jackc/pgx/v5 v5.8.0/go.mod h1:QVeDInX2m9VyzvNeiCJVjCkNFqzsNb43204HshNSZKw=
99
github.com/jackc/puddle/v2 v2.2.2 h1:PR8nw+E/1w0GLuRFSmiioY6UooMp6KJv0/61nB7icHo=
1010
github.com/jackc/puddle/v2 v2.2.2/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4=
1111
github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0=
@@ -14,38 +14,37 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
1414
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
1515
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
1616
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
17-
github.com/riverqueue/river v0.29.0-rc.1 h1:x90jDlMo1NHjBsxIqyzL659DJu/P5LdJI9pptyUQL4A=
18-
github.com/riverqueue/river v0.29.0-rc.1/go.mod h1:Y11Gr3JdZ7eTck7+fO6AOba8rN83QRMzRNkn/cQJeUE=
19-
github.com/riverqueue/river/riverdriver v0.29.0-rc.1 h1:1/PWjCi8pdK5iVdKuwTmpN9tMiOH7a4x/YrpuzbXSsw=
20-
github.com/riverqueue/river/riverdriver v0.29.0-rc.1/go.mod h1:djRhqAz/u/hCo2AEaArHuazNd4LUNGJgj+QqEtB6yRQ=
21-
github.com/riverqueue/river/riverdriver/riverpgxv5 v0.29.0-rc.1 h1:OWT9jx/hRBgt5Jibd2AlzojGkQQvFNRrJ7oxnXzfaAw=
22-
github.com/riverqueue/river/riverdriver/riverpgxv5 v0.29.0-rc.1/go.mod h1:uiS+gn72B+3zrd/5PdQQa6ZbobxHWXywDlr1dBSpb/M=
23-
github.com/riverqueue/river/rivershared v0.29.0-rc.1 h1:3iOgHTTltBk/Gh0HGApsM9P5CbDD4sfbBY/CnnpnWp4=
24-
github.com/riverqueue/river/rivershared v0.29.0-rc.1/go.mod h1:lZkCoXJbadC9smK6/UYmeD+nO7vku4GRQIzH+Y7CTNY=
25-
github.com/riverqueue/river/rivertype v0.29.0-rc.1 h1:FSHV9EUugtOY3nhly9LfxUIOIrS1Sv0TkbKvWQO6940=
26-
github.com/riverqueue/river/rivertype v0.29.0-rc.1/go.mod h1:rWpgI59doOWS6zlVocROcwc00fZ1RbzRwsRTU8CDguw=
17+
github.com/riverqueue/river v0.31.0 h1:BERwce/WS4Guter0/A3GyTDP+1rxl6vFHyBQv+U/5tM=
18+
github.com/riverqueue/river v0.31.0/go.mod h1:Aqbb/jBrFMvh6rbe6SDC6XVZnS0v1W+QQPjejRvyHzk=
19+
github.com/riverqueue/river/riverdriver v0.31.0 h1:XwDa8DqkRxkqMqfdLOYTgSykiTHNSRcWG1LcCg/g0ys=
20+
github.com/riverqueue/river/riverdriver v0.31.0/go.mod h1:Vl6XPbWtjqP+rqEa/HxcEeXeZL/KPCwqjRlqj+wWsq8=
21+
github.com/riverqueue/river/riverdriver/riverpgxv5 v0.31.0 h1:Zii6/VNqasBuPvFIA98xgjz3MRy2EvMm6lMyh1RtWBw=
22+
github.com/riverqueue/river/riverdriver/riverpgxv5 v0.31.0/go.mod h1:z859lpsOraO3IYWjY9w8RZec5I0BAcas9rjZkwxAijU=
23+
github.com/riverqueue/river/rivershared v0.31.0 h1:KVEp+13jnK9YOlMUKnR0eUyJaK+P/APcheoSGMfZArA=
24+
github.com/riverqueue/river/rivershared v0.31.0/go.mod h1:Wvf489bvAiZsJm7mln8YAPZbK7pVfuK7bYfsBt5Nzbw=
25+
github.com/riverqueue/river/rivertype v0.31.0 h1:O6vaJ72SffgF1nxzCrDKd4M+eMZFRlJpycnOcUIGLD8=
26+
github.com/riverqueue/river/rivertype v0.31.0/go.mod h1:D1Ad+EaZiaXbQbJcJcfeicXJMBKno0n6UcfKI5Q7DIQ=
2727
github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8=
2828
github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4=
2929
github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
3030
github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
3131
github.com/tidwall/gjson v1.14.2/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
3232
github.com/tidwall/gjson v1.18.0 h1:FIDeeyB800efLX89e5a8Y0BNH+LOngJyGrIWxG2FKQY=
3333
github.com/tidwall/gjson v1.18.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
34-
github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA=
3534
github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM=
35+
github.com/tidwall/match v1.2.0 h1:0pt8FlkOwjN2fPt4bIl4BoNxb98gGHN2ObFEDkrfZnM=
36+
github.com/tidwall/match v1.2.0/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM=
3637
github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU=
3738
github.com/tidwall/pretty v1.2.1 h1:qjsOFOWWQl+N3RsoF5/ssm1pHmJJwhjlSbZ51I6wMl4=
3839
github.com/tidwall/pretty v1.2.1/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU=
3940
github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY=
4041
github.com/tidwall/sjson v1.2.5/go.mod h1:Fvgq9kS/6ociJEDnK0Fk1cpYF4FIW6ZF7LAe+6jwd28=
4142
go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=
4243
go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE=
43-
golang.org/x/crypto v0.45.0 h1:jMBrvKuj23MTlT0bQEOBcAE0mjg8mK9RXFhRH6nyF3Q=
44-
golang.org/x/crypto v0.45.0/go.mod h1:XTGrrkGJve7CYK7J8PEww4aY7gM3qMCElcJQ8n8JdX4=
45-
golang.org/x/sync v0.18.0 h1:kr88TuHDroi+UVf+0hZnirlk8o8T+4MrK6mr60WkH/I=
46-
golang.org/x/sync v0.18.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI=
47-
golang.org/x/text v0.31.0 h1:aC8ghyu4JhP8VojJ2lEHBnochRno1sgL6nEi9WGFGMM=
48-
golang.org/x/text v0.31.0/go.mod h1:tKRAlv61yKIjGGHX/4tP1LTbc13YSec1pxVEWXzfoeM=
44+
golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4=
45+
golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI=
46+
golang.org/x/text v0.34.0 h1:oL/Qq0Kdaqxa1KbNeMKwQq0reLCCaFtqu2eNuSeNHbk=
47+
golang.org/x/text v0.34.0/go.mod h1:homfLqTYRFyVYemLBFl5GgL/DWEiH5wcsQ5gSh1yziA=
4948
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
5049
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=
5150
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q=

‎riverdriver/riversqlite/river_sqlite_driver.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1626,6 +1626,9 @@ func jobRowFromInternal(internal *dbsqlc.RiverJob) (*rivertype.JobRow, error) {
16261626

16271627
var uniqueStatesByte byte
16281628
if internal.UniqueStates != nil {
1629+
if *internal.UniqueStates < 0 || *internal.UniqueStates > 255 {
1630+
return nil, fmt.Errorf("value out of range for byte: %d", *internal.UniqueStates)
1631+
}
16291632
uniqueStatesByte = byte(*internal.UniqueStates)
16301633
}
16311634

0 commit comments

Comments
 (0)