fix: make AddFixSuggestion thread-safe by Copilot · Pull Request #183 · microsoft/azure-linux-dev-tools

Copilot · 2026-05-12T22:08:41Z

AddFixSuggestion could be called from multiple goroutines, but Env stored suggestions in an unsynchronized slice. This change makes fix suggestion collection safe under concurrent use and preserves suggestions added through WithCancel() child envs.

Shared synchronized storage
- Replace the raw []string on Env with shared fix-suggestion state guarded by a mutex.
- Keep the state shared across shallow Env copies so child envs append into the same suggestion list.
Safe read path
- Snapshot suggestions before printing so formatting/logging never iterates over a slice being mutated concurrently.
Focused regression coverage
- Add tests for concurrent AddFixSuggestion calls on the same env.
- Add tests for concurrent additions from WithCancel() child envs to verify suggestions remain visible on the parent.

childEnv, cancel := testEnv.Env.WithCancel()
defer cancel()

childEnv.AddFixSuggestion("retry with updated inputs")
testEnv.Env.PrintFixSuggestions()

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

dario.cat
- Triggering command: /home/REDACTED/.cache/go-build/7b/7be0cdfd4e5e66df21672375347bd23e46040e0c601b55d1ed8bef64f03acd06-d/go-licenses /home/REDACTED/.cache/go-build/7b/7be0cdfd4e5e66df21672375347bd23e46040e0c601b55d1ed8bef64f03acd06-d/go-licenses report --ignore golang.org/x --ignore github.com/microsoft/azure-linux-dev-tools ./... ux-amd64/pkg/tool/linux_amd64/vet iter.go -ifaceassert l/linux_amd64/vet ux-amd64/pkg/tool/linux_amd64/vet -o /tmp/go-build2551339997/b802/_pkg_.a -trimpath ux-amd64/pkg/tool/linux_amd64/vet -p main -lang=go1.25 ux-amd64/pkg/tool/linux_amd64/vet (dns block)
example.com
- Triggering command: /tmp/go-build329767517/b669/sourceproviders.test /tmp/go-build329767517/b669/sourceproviders.test -test.testlogfile=/tmp/go-build329767517/b669/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build329767517/b558/vet.cfg nal/app/azldev/c-errorsas --pretty=format:-ifaceassert ux-amd64/pkg/tool/linux_amd64/vet -- /home/REDACTED/wor-atomic ux-amd64/pkg/too-bool ToU22LcjRrvL log -dev-tools/magef-errorsas %cd ux-amd64/pkg/tool/linux_amd64/vet -- k/azure-linux-de-atomic ux-amd64/pkg/too-bool ux-amd64/pkg/too-buildtags (dns block)
- Triggering command: /tmp/go-build2950161913/b669/sourceproviders.test /tmp/go-build2950161913/b669/sourceproviders.test -test.testlogfile=/tmp/go-build2950161913/b669/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build2950161913/b510/vet.cfg v-tools/azure-linux-dev-tools/sc-- "||"}} ux-amd64/bin/go GOINSECURE GOMOD GOMODCACHE 9gZq2KlM9N4Y est.�� mds/config GO111MODULE e1778a632bfd545a839f5e2e1069c0a7-d/mockgen GOINSECURE GOMOD nk e1778a632bfd545a-buildtags (dns block)
- Triggering command: /tmp/go-build103592675/b669/sourceproviders.test /tmp/go-build103592675/b669/sourceproviders.test -test.testlogfile=/tmp/go-build103592675/b669/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build103592675/b412/vet.cfg v-tools/azure-linux-dev-tools/in-- d656f853378c1fc3--bundle ux-amd64/bin/go y ternal/app/azlde-V=full nk ortcfg mod estenv nk ux-amd64/bin/go (dns block)
go.opentelemetry.io
- Triggering command: /home/REDACTED/.cache/go-build/7b/7be0cdfd4e5e66df21672375347bd23e46040e0c601b55d1ed8bef64f03acd06-d/go-licenses /home/REDACTED/.cache/go-build/7b/7be0cdfd4e5e66df21672375347bd23e46040e0c601b55d1ed8bef64f03acd06-d/go-licenses report --ignore golang.org/x --ignore github.com/microsoft/azure-linux-dev-tools ./... ux-amd64/pkg/tool/linux_amd64/vet iter.go -ifaceassert l/linux_amd64/vet ux-amd64/pkg/tool/linux_amd64/vet -o /tmp/go-build2551339997/b802/_pkg_.a -trimpath ux-amd64/pkg/tool/linux_amd64/vet -p main -lang=go1.25 ux-amd64/pkg/tool/linux_amd64/vet (dns block)
go.szostok.io
- Triggering command: /home/REDACTED/.cache/go-build/7b/7be0cdfd4e5e66df21672375347bd23e46040e0c601b55d1ed8bef64f03acd06-d/go-licenses /home/REDACTED/.cache/go-build/7b/7be0cdfd4e5e66df21672375347bd23e46040e0c601b55d1ed8bef64f03acd06-d/go-licenses report --ignore golang.org/x --ignore github.com/microsoft/azure-linux-dev-tools ./... ux-amd64/pkg/tool/linux_amd64/vet iter.go -ifaceassert l/linux_amd64/vet ux-amd64/pkg/tool/linux_amd64/vet -o /tmp/go-build2551339997/b802/_pkg_.a -trimpath ux-amd64/pkg/tool/linux_amd64/vet -p main -lang=go1.25 ux-amd64/pkg/tool/linux_amd64/vet (dns block)
go.uber.org
- Triggering command: /home/REDACTED/.cache/go-build/7b/7be0cdfd4e5e66df21672375347bd23e46040e0c601b55d1ed8bef64f03acd06-d/go-licenses /home/REDACTED/.cache/go-build/7b/7be0cdfd4e5e66df21672375347bd23e46040e0c601b55d1ed8bef64f03acd06-d/go-licenses report --ignore golang.org/x --ignore github.com/microsoft/azure-linux-dev-tools ./... ux-amd64/pkg/tool/linux_amd64/vet iter.go -ifaceassert l/linux_amd64/vet ux-amd64/pkg/tool/linux_amd64/vet -o /tmp/go-build2551339997/b802/_pkg_.a -trimpath ux-amd64/pkg/tool/linux_amd64/vet -p main -lang=go1.25 ux-amd64/pkg/tool/linux_amd64/vet (dns block)
go.yaml.in
- Triggering command: /home/REDACTED/.cache/go-build/7b/7be0cdfd4e5e66df21672375347bd23e46040e0c601b55d1ed8bef64f03acd06-d/go-licenses /home/REDACTED/.cache/go-build/7b/7be0cdfd4e5e66df21672375347bd23e46040e0c601b55d1ed8bef64f03acd06-d/go-licenses report --ignore golang.org/x --ignore github.com/microsoft/azure-linux-dev-tools ./... ux-amd64/pkg/tool/linux_amd64/vet iter.go -ifaceassert l/linux_amd64/vet ux-amd64/pkg/tool/linux_amd64/vet -o /tmp/go-build2551339997/b802/_pkg_.a -trimpath ux-amd64/pkg/tool/linux_amd64/vet -p main -lang=go1.25 ux-amd64/pkg/tool/linux_amd64/vet (dns block)
gopkg.in
- Triggering command: /home/REDACTED/.cache/go-build/7b/7be0cdfd4e5e66df21672375347bd23e46040e0c601b55d1ed8bef64f03acd06-d/go-licenses /home/REDACTED/.cache/go-build/7b/7be0cdfd4e5e66df21672375347bd23e46040e0c601b55d1ed8bef64f03acd06-d/go-licenses report --ignore golang.org/x --ignore github.com/microsoft/azure-linux-dev-tools ./... ux-amd64/pkg/tool/linux_amd64/vet iter.go -ifaceassert l/linux_amd64/vet ux-amd64/pkg/tool/linux_amd64/vet -o /tmp/go-build2551339997/b802/_pkg_.a -trimpath ux-amd64/pkg/tool/linux_amd64/vet -p main -lang=go1.25 ux-amd64/pkg/tool/linux_amd64/vet (dns block)
invalid-domain.there.is.no.such.resource
- Triggering command: /tmp/go-build329767517/b662/rpmprovider.test /tmp/go-build329767517/b662/rpmprovider.test -test.testlogfile=/tmp/go-build329767517/b662/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build329767517/b549/vet.cfg nal/utils/parmap-errorsas --pretty=format:-ifaceassert ux-amd64/pkg/tool/linux_amd64/vet -- /home/REDACTED/wor-atomic ux-amd64/pkg/too-bool ux-amd64/pkg/too-buildtags tbui�� -dev-tools/inter-errorsas GO111MODULE ux-amd64/pkg/tool/linux_amd64/vet GOINSECURE GOMOD GOMODCACHE ux-amd64/pkg/too-buildtags (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Agent-Logs-Url: https://github.com/microsoft/azure-linux-dev-tools/sessions/5c16317f-6885-4df7-8091-874f9b19917c Co-authored-by: dmcilvaney <23200982+dmcilvaney@users.noreply.github.com>

dmcilvaney · 2026-05-12T23:16:05Z

@copilot What drove the decision to have a new type rather than just adding a dedicated mutex to env that protects the hints? Pros/Cons of each?

Copilot · 2026-05-12T23:17:04Z

@copilot What drove the decision to have a new type rather than just adding a dedicated mutex to env that protects the hints? Pros/Cons of each?

The main driver was Env.WithCancel(): it does a shallow copy of Env, so a mutex stored directly on Env would also get copied. That creates a footgun where parent/child envs no longer synchronize on the same lock, even though they need to share the same fix-suggestion state. Using a dedicated fixSuggestionState pointer let the shallow copies share both the slice and the mutex together, and it keeps the locking logic localized to the hint storage itself.

Pros of the current approach: safe with WithCancel(), encapsulates the synchronization with the data it protects, and makes future fix-suggestion operations easier to keep correct. Cons: one extra type and an extra indirection. A mutex directly on Env would be a little flatter/simpler, but only if we also reworked WithCancel()/state sharing carefully so we didn’t copy the mutex or split the protected state. Relevant change: f6ff539.

dmcilvaney · 2026-05-12T23:30:43Z

+	state.suggestions = append(state.suggestions, suggestion)
+}
+
+func (state *fixSuggestionState) Snapshot() []string {


@copilot Can we use a more simple name here?

Also, please add brief comments to these since they are public.

Updated in 38f5a58: renamed Snapshot() to All() and added brief doc comments to the helper methods.

Agent-Logs-Url: https://github.com/microsoft/azure-linux-dev-tools/sessions/74c9103d-8308-4e14-a8f4-abbb5d53d3fc Co-authored-by: dmcilvaney <23200982+dmcilvaney@users.noreply.github.com>

Copilot · 2026-05-12T23:36:06Z

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

example.com
- Triggering command: /tmp/go-build865071806/b669/sourceproviders.test /tmp/go-build865071806/b669/sourceproviders.test -test.testlogfile=/tmp/go-build865071806/b669/testlog.txt -test.paniconexit0 -test.timeout=10m0s o_.o�� u/13/liblto_plug-errorsas 0.1-go1.25.6.lin-ifaceassert ux-amd64/pkg/tool/linux_amd64/vet --gdwarf-5 s ead gEwj5G1E-cvT -I v0.6.0/sha1cdblo-errorsas mpile ux-amd64/pkg/tool/linux_amd64/vet --gdwarf-5 fips140/nistec/f-o -o ux-amd64/pkg/too-trimpath (dns block)
- Triggering command: /tmp/go-build3717345813/b669/sourceproviders.test /tmp/go-build3717345813/b669/sourceproviders.test -test.testlogfile=/tmp/go-build3717345813/b669/testlog.txt -test.paniconexit0 -test.timeout=10m0s go1.25.6 -c=4 -nolocalimports -importcfg /tmp/go-build3717345813/b002/importcfg -pack /home/REDACTED/work/azure-linux-dev-tools/azure-linux-dev-tools/pkg/app/azldev_cli/azldev.go 0.1-go1.25.6.lincontext (dns block)
invalid-domain.there.is.no.such.resource
- Triggering command: /tmp/go-build865071806/b662/rpmprovider.test /tmp/go-build865071806/b662/rpmprovider.test -test.testlogfile=/tmp/go-build865071806/b662/testlog.txt -test.paniconexit0 -test.timeout=10m0s go1.25.6 -c=4 -nolocalimports -importcfg /tmp/go-build865071806/b579/importcfg -pack /home/REDACTED/work/azure-linux-dev-tools/azure-linux-dev-tools/internal/app/azldev/core/testutils/cobrautils.go /home/REDACTED/work/azure-linux-dev-tools/azure-linux-dev-tools/internal/app/azldev/core/testutils/testenv.go -I go-crypto@v1.1.6-errorsas go-crypto@v1.1.6-ifaceassert ux-amd64/pkg/tool/linux_amd64/vet --gdwarf-5 s ead ux-amd64/pkg/too-buildtags (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Copilot

Pull request overview

This PR addresses a concurrency hazard in azldev.Env fix-suggestion tracking by replacing the unsynchronized []string accumulator with a shared, mutex-guarded state that remains shared across WithCancel()-derived child envs. It also updates the printing path to snapshot suggestions before iterating, preventing concurrent mutation while formatting/logging.

Changes:

Replace Env.fixSuggestions []string with a shared *fixSuggestionState guarded by sync.Mutex to make AddFixSuggestion safe under concurrent use.
Update PrintFixSuggestions to read a snapshot (All()) before iterating/logging.
Add regression tests covering concurrent AddFixSuggestion and additions from WithCancel() child envs.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
internal/app/azldev/env.go	Introduces mutex-guarded shared fix-suggestion storage and snapshot-based printing to prevent data races.
internal/app/azldev/env_test.go	Adds concurrency regression tests validating thread-safe suggestion collection and parent visibility from child envs.

Initial plan

9065581

Copilot AI assigned Copilot and dmcilvaney May 12, 2026

Copilot AI requested review from Copilot and removed request for Copilot May 12, 2026 22:08

Copilot started work on behalf of dmcilvaney May 12, 2026 22:08 View session

Copilot AI linked an issue May 12, 2026 that may be closed by this pull request

AddFixSuggestion should be made thread-safe #182

Open

fix: make fix suggestion storage thread-safe

f6ff539

Agent-Logs-Url: https://github.com/microsoft/azure-linux-dev-tools/sessions/5c16317f-6885-4df7-8091-874f9b19917c Co-authored-by: dmcilvaney <23200982+dmcilvaney@users.noreply.github.com>

Copilot AI requested review from Copilot and removed request for Copilot May 12, 2026 22:17

test: polish concurrent fix suggestion coverage

6737093

Agent-Logs-Url: https://github.com/microsoft/azure-linux-dev-tools/sessions/5c16317f-6885-4df7-8091-874f9b19917c Co-authored-by: dmcilvaney <23200982+dmcilvaney@users.noreply.github.com>

Copilot AI requested review from Copilot and removed request for Copilot May 12, 2026 22:21

refactor: simplify fix suggestion state helper

4b570a4

Agent-Logs-Url: https://github.com/microsoft/azure-linux-dev-tools/sessions/5c16317f-6885-4df7-8091-874f9b19917c Co-authored-by: dmcilvaney <23200982+dmcilvaney@users.noreply.github.com>

Copilot AI requested review from Copilot and removed request for Copilot May 12, 2026 22:23

test: simplify logger restoration helper

63b8c60

Agent-Logs-Url: https://github.com/microsoft/azure-linux-dev-tools/sessions/5c16317f-6885-4df7-8091-874f9b19917c Co-authored-by: dmcilvaney <23200982+dmcilvaney@users.noreply.github.com>

Copilot AI requested review from Copilot and removed request for Copilot May 12, 2026 22:25

Copilot AI changed the title ~~[WIP] Fix AddFixSuggestion to be thread-safe~~ fix: make AddFixSuggestion thread-safe May 12, 2026

Copilot finished work on behalf of dmcilvaney May 12, 2026 22:26

Copilot AI requested a review from dmcilvaney May 12, 2026 22:26

Copilot started work on behalf of dmcilvaney May 12, 2026 23:16 View session

Copilot finished work on behalf of dmcilvaney May 12, 2026 23:17

dmcilvaney reviewed May 12, 2026

View reviewed changes

Copilot started work on behalf of dmcilvaney May 12, 2026 23:30 View session

refactor: simplify fix suggestion helper naming

38f5a58

Agent-Logs-Url: https://github.com/microsoft/azure-linux-dev-tools/sessions/74c9103d-8308-4e14-a8f4-abbb5d53d3fc Co-authored-by: dmcilvaney <23200982+dmcilvaney@users.noreply.github.com>

Copilot AI review requested due to automatic review settings May 12, 2026 23:35

Copilot finished work on behalf of dmcilvaney May 12, 2026 23:36

Copilot AI requested a review from dmcilvaney May 12, 2026 23:36

dmcilvaney approved these changes May 13, 2026

View reviewed changes

dmcilvaney marked this pull request as ready for review May 13, 2026 21:55

Copilot AI review requested due to automatic review settings May 13, 2026 21:55

Copilot started reviewing on behalf of dmcilvaney May 13, 2026 21:57 View session

Copilot AI reviewed May 13, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: make AddFixSuggestion thread-safe#183

fix: make AddFixSuggestion thread-safe#183
Copilot wants to merge 6 commits into
mainfrom
copilot/make-addfixsuggestion-thread-safe

Copilot AI commented May 12, 2026 •

edited

Loading

Uh oh!

dmcilvaney commented May 12, 2026

Uh oh!

Copilot AI commented May 12, 2026

Uh oh!

dmcilvaney May 12, 2026

Uh oh!

Copilot AI May 12, 2026

Uh oh!

Copilot AI commented May 12, 2026

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Copilot AI commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

dmcilvaney commented May 12, 2026

Uh oh!

Copilot AI commented May 12, 2026

Uh oh!

dmcilvaney May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented May 12, 2026

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented May 12, 2026 •

edited

Loading