Skip to content

Fix data race in 'discard connections' pool test. [v1] #1877

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 24, 2024
Merged

Fix data race in 'discard connections' pool test. [v1] #1877

merged 1 commit into from
Oct 24, 2024
+2 −2

Conversation

matthewdale
Copy link
Collaborator

Based on #1860

Summary

  • Don't share the err variable in the goroutine closure (pool_test.go line 893) to prevent a data race.
  • Enable the race detector in the test-short Taskfile target.

Background & Motivation

The race detector panics because of a data race in the "discards connections closed by the server side" connection pool test:

WARNING: DATA RACE
Read at 0x00c0005f4aa0 by goroutine 4142:
  go.mongodb.org/mongo-driver/v2/x/mongo/driver/topology.TestPool_checkOut.func10.3()
      /Users/matt.dale/work/mongo-go-driver/x/mongo/driver/topology/pool_test.go:897 +0x1b0

Previous write at 0x00c0005f4aa0 by goroutine 3796:
  go.mongodb.org/mongo-driver/v2/x/mongo/driver/topology.TestPool_checkOut.func10()
      /Users/matt.dale/work/mongo-go-driver/x/mongo/driver/topology/pool_test.go:907 +0x90c
...

@matthewdale
Fix data race in 'discard connections' pool test. [v1]

Verified

This commit was signed with the committer’s verified signature.
BiagioFesta Biagio Festa
SSH Key Fingerprint: mc9oOaotR5c8rI/ox0nGJR5a9v1sb6us864SIq8Iw+o
Verified
Learn about vigilant mode
Loading
Loading status checks…
3dd4062
@mongodb-drivers-pr-bot mongodb-drivers-pr-bot bot added the priority-3-low Low Priority PR for Review label Oct 23, 2024
@mongodb-drivers-pr-bot mongodb-drivers-pr-bot
Copy link
Contributor

API Change Report

No changes found!

@blink1073 blink1073 requested review from qingyang-hu and removed request for blink1073 October 24, 2024 00:34
@matthewdale matthewdale merged commit e4c2740 into mongodb:v1 Oct 24, 2024
29 of 33 checks passed
matthewdale added a commit that referenced this pull request Oct 24, 2024
@matthewdale
Fix data race in 'discard connections' pool test. [v1] (#1877)

Verified

This commit was signed with the committer’s verified signature.
BiagioFesta Biagio Festa
SSH Key Fingerprint: mc9oOaotR5c8rI/ox0nGJR5a9v1sb6us864SIq8Iw+o
Verified
Learn about vigilant mode
32067cf
@mongodb-drivers-pr-bot mongodb-drivers-pr-bot bot mentioned this pull request Mar 4, 2025
Merged
alcaeus added a commit that referenced this pull request Mar 4, 2025
@alcaeus
Mark release/1.17 as fully merged into v1
Loading
Loading status checks…
2fbd242 
* release/1.17:
  Use different credentials for merge-up PRs (#1968)
  Add GitHub Actions workflow for merge ups (#1962)
  BUMP v1.17.3
  GODRIVER-3448 Limit GOMAXPROCS for fuzz tests (#1939) [v1] (#1943) [release/1.17] (#1957)
  Update reviewers.txt (#1855) [v1] (#1883) [release/1.17] (#1958)
  Cherry pick 1.17.2 work to release/1.17 (#1956)
  GODRIVER-3470 Correct BSON unmarshaling logic for null values (#1924) (#1955)
  GODRIVER-3370 Add bypassEmptyTsReplacement option. (#1927) [release/1.17] (#1954)
  GODRIVER-3340 Bump github.com/klauspost/compress from 1.13.6 to 1.16.7 [release/1.17] (#1880)
  GODRIVER-3374 Add ReadCompressedCompressedMessage back to wiremessage API (#1870)
  Fix data race in 'discard connections' pool test. [v1] (#1877)
  Bump golangci-lint for 1.23 compatibility [v1] (#1875)
  BUMP v1.17.1
  GODRIVER-3156 Detect and discard closed idle connections. (#1815) [release/1.17] (#1841)
  GODRIVER-3313 [release/1.17] Skip CSOT spec tests on Windows and macOS. (#1838)
  GODRIVER-3358 [release/1.17] Do not override authSource from TXT record (#1840)
  GODRIVER-2589 [release/1.17] Clarify `*Cursor.All()` behavior in comment. (#1839)
  DEVPROD-10453 Use assume_role for s3 uploads [release/1.17] (#1824) (#1837)
  update repo metadata
alcaeus added a commit that referenced this pull request Mar 4, 2025
@alcaeus
Mark release/1.17 as fully merged into v1
969166c 
* commit '9c485751':
  BUMP v1.17.3
  GODRIVER-3448 Limit GOMAXPROCS for fuzz tests (#1939) [v1] (#1943) [release/1.17] (#1957)
  Update reviewers.txt (#1855) [v1] (#1883) [release/1.17] (#1958)
  Cherry pick 1.17.2 work to release/1.17 (#1956)
  GODRIVER-3470 Correct BSON unmarshaling logic for null values (#1924) (#1955)
  GODRIVER-3370 Add bypassEmptyTsReplacement option. (#1927) [release/1.17] (#1954)
  GODRIVER-3340 Bump github.com/klauspost/compress from 1.13.6 to 1.16.7 [release/1.17] (#1880)
  GODRIVER-3374 Add ReadCompressedCompressedMessage back to wiremessage API (#1870)
  Fix data race in 'discard connections' pool test. [v1] (#1877)
  Bump golangci-lint for 1.23 compatibility [v1] (#1875)
  BUMP v1.17.1
  GODRIVER-3156 Detect and discard closed idle connections. (#1815) [release/1.17] (#1841)
  GODRIVER-3313 [release/1.17] Skip CSOT spec tests on Windows and macOS. (#1838)
  GODRIVER-3358 [release/1.17] Do not override authSource from TXT record (#1840)
  GODRIVER-2589 [release/1.17] Clarify `*Cursor.All()` behavior in comment. (#1839)
  DEVPROD-10453 Use assume_role for s3 uploads [release/1.17] (#1824) (#1837)
  update repo metadata
@mongodb-drivers-pr-bot mongodb-drivers-pr-bot bot mentioned this pull request Mar 4, 2025
Merged
alcaeus added a commit that referenced this pull request Mar 4, 2025
@alcaeus
Mark v1 as fully merged into release/2.0
f74a8dc 
* commit '0dc2e05e': (184 commits)
  GODRIVER-3448 Limit GOMAXPROCS for fuzz tests (#1939) [v1] (#1943)
  GODRIVER-3470 Correct BSON unmarshaling logic for null values (#1924)
  GODRIVER-3370 Add bypassEmptyTsReplacement option. (#1927)
  BUMP v1.17.2
  GODRIVER-3436 Avoid initializing null data given custom decoder (#1902)
  GODRIVER-3340 Add a test for goroutine leaks. (#1874)
  Update reviewers.txt (#1855) [v1] (#1883)
  Fix data race in 'discard connections' pool test. [v1] (#1877)
  Bump golangci-lint for 1.23 compatibility [v1] (#1875)
  GODRIVER-3340 Bump github.com/klauspost/compress from 1.13.6 to 1.16.7 [v1] (#1869)
  GODRIVER-3374 Add ReadCompressedCompressedMessage back to wiremessage API (#1870)
  GODRIVER-3156 Detect and discard closed idle connections. (#1815)
  GODRIVER-3358 Do not override authSource from TXT record (#1830)
  DEVPROD-10453 Use assume_role for s3 uploads [v1] (#1824)
  GODRIVER-2589 Clarify `*Cursor.All()` behavior in comment. (#1804)
  GODRIVER-3313 Skip CSOT spec tests on Windows and macOS. (#1818)
  BUMP v1.17.0
  GODRIVER-3302 Handle malformatted message length properly. (#1758)
  GODRIVER-3312 Use remaining test secrets from the vault [v1] (#1811)
  Remove GCP from supplied callback example (#1809)
  ...
alcaeus added a commit that referenced this pull request Mar 4, 2025
@alcaeus
Merge branch 'v1' into merge-v1-into-release/2.0-1741093165191
Loading
Loading status checks…
81d33e6 
* v1:
  Use different credentials for merge-up PRs (#1968)
  Add GitHub Actions workflow for merge ups (#1962)
  BUMP v1.17.3
  GODRIVER-3448 Limit GOMAXPROCS for fuzz tests (#1939) [v1] (#1943) [release/1.17] (#1957)
  Update reviewers.txt (#1855) [v1] (#1883) [release/1.17] (#1958)
  Cherry pick 1.17.2 work to release/1.17 (#1956)
  GODRIVER-3470 Correct BSON unmarshaling logic for null values (#1924) (#1955)
  GODRIVER-3370 Add bypassEmptyTsReplacement option. (#1927) [release/1.17] (#1954)
  GODRIVER-3340 Bump github.com/klauspost/compress from 1.13.6 to 1.16.7 [release/1.17] (#1880)
  GODRIVER-3374 Add ReadCompressedCompressedMessage back to wiremessage API (#1870)
  Fix data race in 'discard connections' pool test. [v1] (#1877)
  Bump golangci-lint for 1.23 compatibility [v1] (#1875)
  BUMP v1.17.1
  GODRIVER-3156 Detect and discard closed idle connections. (#1815) [release/1.17] (#1841)
  GODRIVER-3313 [release/1.17] Skip CSOT spec tests on Windows and macOS. (#1838)
  GODRIVER-3358 [release/1.17] Do not override authSource from TXT record (#1840)
  GODRIVER-2589 [release/1.17] Clarify `*Cursor.All()` behavior in comment. (#1839)
  DEVPROD-10453 Use assume_role for s3 uploads [release/1.17] (#1824) (#1837)
  update repo metadata
@mongodb-drivers-pr-bot mongodb-drivers-pr-bot bot mentioned this pull request Mar 4, 2025
Merged
alcaeus added a commit that referenced this pull request Mar 4, 2025
@alcaeus
Merge branch 'release/2.0' into merge-release/2.0-into-release/2.1-17…
Loading
Loading status checks…
8ea0039 
…41094483929

* release/2.0: (203 commits)
  Use different credentials for merge-up PRs (#1968)
  Add GitHub Actions workflow for merge ups (#1962)
  BUMP v1.17.3
  GODRIVER-3448 Limit GOMAXPROCS for fuzz tests (#1939) [v1] (#1943) [release/1.17] (#1957)
  Update reviewers.txt (#1855) [v1] (#1883) [release/1.17] (#1958)
  Cherry pick 1.17.2 work to release/1.17 (#1956)
  GODRIVER-3470 Correct BSON unmarshaling logic for null values (#1924) (#1955)
  GODRIVER-3370 Add bypassEmptyTsReplacement option. (#1927) [release/1.17] (#1954)
  GODRIVER-3448 Limit GOMAXPROCS for fuzz tests (#1939) [v1] (#1943)
  GODRIVER-3470 Correct BSON unmarshaling logic for null values (#1924)
  GODRIVER-3370 Add bypassEmptyTsReplacement option. (#1927)
  BUMP v1.17.2
  GODRIVER-3436 Avoid initializing null data given custom decoder (#1902)
  GODRIVER-3340 Add a test for goroutine leaks. (#1874)
  Update reviewers.txt (#1855) [v1] (#1883)
  GODRIVER-3340 Bump github.com/klauspost/compress from 1.13.6 to 1.16.7 [release/1.17] (#1880)
  GODRIVER-3374 Add ReadCompressedCompressedMessage back to wiremessage API (#1870)
  Fix data race in 'discard connections' pool test. [v1] (#1877)
  Bump golangci-lint for 1.23 compatibility [v1] (#1875)
  Fix data race in 'discard connections' pool test. [v1] (#1877)
  ...
@mongodb-drivers-pr-bot mongodb-drivers-pr-bot bot mentioned this pull request Mar 4, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qingyang-hu qingyang-hu

Assignees
No one assigned
Labels
priority-3-low Low Priority PR for Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@matthewdale @qingyang-hu