Skip to content

Conversation

@Pnkcaht
Copy link
Contributor

@Pnkcaht Pnkcaht commented Jan 21, 2026

What I did

  • Protected the Config.Aliases map with a mutex.
  • Made all reads and writes to aliases safe for concurrent use.
  • Added minimal documentation explaining the concurrency guarantees.

Related issue

Fixes #1454

What was the bug

The Config.Aliases map was accessed concurrently without synchronization.

When multiple goroutines (or parallel tests) called methods like SetAlias, GetAlias, or DeleteAlias at the same time, this resulted in:

  • fatal error: concurrent map writes
  • flaky test failures in CI

Because Go maps are not thread-safe, even concurrent reads and writes could trigger panics.

Description

In the previous implementation, the Config struct exposed methods that mutated or read from the Aliases map without any form of synchronization.

When these methods were invoked concurrently—most notably from tests using t.Parallel()—the runtime detected unsafe concurrent access and crashed.

This change introduces a mutex inside Config and ensures that all accesses to the Aliases map are properly synchronized.

Result

  • Alias operations are now safe for concurrent use.
  • Flaky panics caused by concurrent map access are eliminated.
  • No behavioral or API changes were introduced.

Validation — Reproducing the original failure

The issue was originally reproducible by repeatedly running the userconfig test suite, which intermittently failed with a concurrent map access panic.

image

Validation — Race detector

The fix was also validated using the Go race detector:

image

Validation — Full build

After the fix, the full test suite (go test ./...) runs successfully without failures.

Signed-off-by: pnkcaht <samzoovsk19@gmail.com>
@Pnkcaht Pnkcaht requested a review from a team as a code owner January 21, 2026 00:21
@Pnkcaht Pnkcaht mentioned this pull request Jan 21, 2026
Copy link
Member

@rumpl rumpl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

neat

@dgageot
Copy link
Member

dgageot commented Jan 21, 2026

@Pnkcaht we have a concurrent.Map for that

@rumpl
Copy link
Member

rumpl commented Jan 21, 2026

@Pnkcaht we have a concurrent.Map for that

The concurrent.Map can't be marshalled @dgageot

@dgageot dgageot merged commit 500fe59 into docker:main Jan 21, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test failure (flaky)

3 participants