Skip to content

fix: hold RLock during copy in Get to prevent concurrent map access - #406

Merged
knadh merged 1 commit into
knadh:masterfrom
alexchenai:fix/get-concurrent-mutex-race-condition
Mar 20, 2026
Merged

fix: hold RLock during copy in Get to prevent concurrent map access#406
knadh merged 1 commit into
knadh:masterfrom
alexchenai:fix/get-concurrent-mutex-race-condition

Conversation

@alexchenai

Copy link
Copy Markdown
Contributor

Summary

  • Fixes a race condition in Get() where the RLock was released before maps.Copy() and copystructure.Copy() operated on reference types (maps, slices, etc.), allowing concurrent writes to cause fatal error: concurrent map read and map write panics.
  • Uses defer ko.mu.RUnlock() so the read lock is held for the entire duration of the copy operations, matching the pattern already used by All() and Raw().
  • Also fixes a minor typo in a comment ("Skil" -> "Skip").

Details

The Get method previously did:

ko.mu.RLock()
res := maps.Search(ko.confMap, p)
ko.mu.RUnlock()       // <-- lock released here
// ...
maps.Copy(v)          // <-- iterates map without lock protection
copystructure.Copy()  // <-- same issue

This created a window where a concurrent Load/Set/Delete could modify the underlying map while Copy was iterating it. The fix changes to defer ko.mu.RUnlock() so the lock covers the copy operations.

Fixes #401

Test plan

  • go vet ./... passes
  • go build ./... passes
  • Existing tests pass
  • The race condition requires go test -race with CGO enabled to reproduce reliably

🤖 Generated with Claude Code

…ap access

The Get method released RLock before calling maps.Copy and
copystructure.Copy on reference types (maps, slices, etc.), creating a
race condition where concurrent writes could modify the underlying data
during the copy. This caused "concurrent map read and map write" panics.

Use defer to keep the read lock held until the copy completes.

Also fixes a typo: "Skil" -> "Skip" in a comment.

Fixes knadh#401

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@knadh
knadh merged commit f394588 into knadh:master Mar 20, 2026
3 checks passed
Maks1mS pushed a commit to stplr-dev/stplr that referenced this pull request Mar 22, 2026
This PR contains the following updates:

| Package | Type | Update | Change | OpenSSF |
|---|---|---|---|---|
| [github.com/knadh/koanf/v2](https://github.com/knadh/koanf) | require | patch | `v2.3.3` → `v2.3.4` | [![OpenSSF Scorecard](https://api.securityscorecards.dev/projects/github.com/knadh/koanf/badge)](https://securityscorecards.dev/viewer/?uri=github.com/knadh/koanf) |

---

> ⚠️ **Warning**
>
> Some dependencies could not be looked up. Check the [Dependency Dashboard](issues/23) for more information.

---

### Release Notes

<details>
<summary>knadh/koanf (github.com/knadh/koanf/v2)</summary>

### [`v2.3.4`](https://github.com/knadh/koanf/releases/tag/v2.3.4)

[Compare Source](knadh/koanf@v2.3.3...v2.3.4)

#### What's Changed

- Bump github.com/nats-io/nats-server/v2 from 2.10.27 to 2.11.12 in /providers/nats by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;400](knadh/koanf#400)
- Bump filippo.io/edwards25519 from 1.1.0 to 1.1.1 in /providers/kiln by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;399](knadh/koanf#399)
- Bump google.golang.org/grpc from 1.71.1 to 1.79.3 in /providers/etcd by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;405](knadh/koanf#405)
- fix: hold RLock during copy in Get to prevent concurrent map access by [@&#8203;alexchenai](https://github.com/alexchenai) in [#&#8203;406](knadh/koanf#406)
- Add ability to check for prior values in `cliflagv3.ProviderWithConfig()` just like `posflag` by [@&#8203;knadh](https://github.com/knadh) in [#&#8203;403](knadh/koanf#403)

#### New Contributors

- [@&#8203;alexchenai](https://github.com/alexchenai) made their first contribution in [#&#8203;406](knadh/koanf#406)

**Full Changelog**: <knadh/koanf@v2.3.3...v2.3.4>

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At 12:00 AM through 04:59 AM and 10:00 PM through 11:59 PM, Monday through Friday ( * 0-4,22-23 * * 1-5 ), Only on Sunday and Saturday ( * * * * 0,6 ) (UTC), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My43Ni4zIiwidXBkYXRlZEluVmVyIjoiNDMuNzYuMyIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiS2luZC9EZXBlbmRlbmNpZXMiXX0=-->

Reviewed-on: https://altlinux.space/stapler/stplr/pulls/370
Co-authored-by: Renovate Bot <[email protected]>
Co-committed-by: Renovate Bot <[email protected]>
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.

Get: concurrent mutex bug

2 participants