Fix concurrent map writes in codespace port forwarding by williammartin · Pull Request #13313 · cli/cli

williammartin · 2026-04-29T15:34:17Z

When forwarding multiple ports concurrently with gh cs ports forward, the dev-tunnels TunnelManager mutates shared state on the Tunnel object in buildUri(), which is not goroutine-safe. This causes a fatal error: concurrent map writes panic.

Root cause

ForwardPorts runs all port forwards concurrently via errgroup, sharing a single CodespaceConnection. The shallow copy in NewPortForwarder (*codespaceConnection dereference) means all forwarders share the same TunnelManager and Tunnel pointers, so concurrent calls to GetTunnelPort / CreateTunnelPort race on the shared map.

Fix

Store connection as a pointer - changed CodespacesPortForwarder.connection from connection.CodespaceConnection (value) to *connection.CodespaceConnection (pointer), eliminating the unnecessary shallow copy. Since all the important fields (TunnelManager, TunnelClient, Tunnel, Options) were already pointers, the copy was pointless and created this class of bug.
Added sync.Mutex (ManagerMu) to CodespaceConnection to serialize TunnelManager operations that mutate shared state on the Tunnel object. Because the connection is now shared by pointer, a plain sync.Mutex (not a pointer) works naturally.
Extracted createTunnelPort helper from ForwardPort so the mutex-guarded critical section uses defer to unlock, replacing fragile manual Unlock() calls on each error path.
Connect and RefreshPorts already have their own synchronization and are left outside the lock.

Test

Added TestConcurrentForwardPortDoesNotRace which forwards 10 ports concurrently from a shared connection, reproducing the race detected by go test -race.

Performance

The mutex serializes TunnelManager API calls that were previously concurrent. Benchmarking with a real codespace (5 ports, 5 runs each) shows no measurable impact:

	Run 1	Run 2	Run 3	Run 4	Run 5	Avg
Stock CLI (v2.92.0)	1.03s	0.89s	0.90s	0.91s	1.18s	0.98s
Fixed CLI	1.04s	1.04s	0.89s	0.98s	1.08s	1.01s

The serialization overhead is negligible because the bottleneck is network round-trips to the dev-tunnels API and SSH tunnel setup, not CPU time in the critical section.

Co-authored-by: Copilot <[email protected]>

Instead of copying CodespaceConnection by value and using a *sync.Mutex pointer to share the mutex across copies, store a *CodespaceConnection pointer in CodespacesPortForwarder. This makes all forwarders naturally share the same connection (and its mutex) without pointer tricks. Also extract the mutex-guarded section in ForwardPort into a helper method (createTunnelPort) that uses defer to unlock, replacing fragile manual Unlock calls on each error path. Co-authored-by: Copilot <[email protected]>

Co-authored-by: Copilot <[email protected]>

Copilot

Pull request overview

Fixes a race in gh cs ports forward where concurrent port forwards can trigger fatal error: concurrent map writes due to goroutine-unsafe mutations inside the dev-tunnels TunnelManager/Tunnel objects.

Changes:

Switch CodespacesPortForwarder.connection to store *connection.CodespaceConnection (avoids shallow-copying shared pointers).
Add ManagerMu sync.Mutex to CodespaceConnection and use it to serialize TunnelManager operations (create/get/list/delete ports).
Add a concurrent forwarding test intended to reproduce the race under go test -race.

Show a summary per file

File	Description
internal/codespaces/portforwarder/port_forwarder.go	Avoids connection shallow-copying and serializes tunnel-manager operations with a mutex.
internal/codespaces/portforwarder/port_forwarder_test.go	Adds/updates tests, including a new concurrent-forwarding regression test.
internal/codespaces/connection/connection.go	Introduces `ManagerMu` on `CodespaceConnection` to coordinate manager access across forwarders.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 3/3 changed files
Comments generated: 3

Wrap GetTunnelPort and DeleteTunnelPort calls in anonymous functions that use defer for the mutex unlock, making them robust against future edits that add early returns between Lock and Unlock. Co-authored-by: Copilot <[email protected]>

- Hold ManagerMu across the entire get/check/delete sequence in UpdatePortVisibility to eliminate a time-of-check-time-of-use gap - Remove IIFE pattern in favor of explicit lock/unlock - Revert signal.NotifyContext change in cmd.go (out of scope) Co-authored-by: Copilot <[email protected]>

Cleans up an accidental blank line splitting the stdlib imports in cmd.go. Co-authored-by: Copilot App <[email protected]>

BagToad

LGTM

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [cli/cli](https://github.com/cli/cli) | minor | `v2.94.0` → `v2.96.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>cli/cli (cli/cli)</summary> ### [`v2.96.0`](https://github.com/cli/cli/releases/tag/v2.96.0): GitHub CLI 2.96.0 [Compare Source](cli/cli@v2.95.0...v2.96.0) #### Security A security vulnerability has been identified, and fixed, that could allow command execution on a user's computer when connecting to a malicious Codespace via `gh codespace jupyter`. Users of `gh codespace jupyter` are advised to update gh to version v2.96.0 as soon as possible. For more information see: <GHSA-8cg3-r6g9-fpg2> #### Download release assets without authentication `gh release download` now works against public repositories without authentication, matching `gh extension install`. A token is still used when one is present: ```shell # Download assets from a public repository, no login required gh release download v2.96.0 --repo cli/cli ``` #### What's Changed ##### ✨ Features - Allow `gh release download` without authentication on public repositories by [@BagToad](https://github.com/BagToad) in [#13723](cli/cli#13723) - Detect additional third-party coding agents by [@BagToad](https://github.com/BagToad) in [#13722](cli/cli#13722) - Support `antigravity-cli` and `antigravity2.0` in `gh skill` by [@BagToad](https://github.com/BagToad) in [#13784](cli/cli#13784) ##### 🐛 Fixes - fix: show checks summary when all checks were cancelled by [@s3onghyun](https://github.com/s3onghyun) in [#13679](cli/cli#13679) - fix(skills): install universal agent to `~/.agents/skills` by [@toller892](https://github.com/toller892) in [#13681](cli/cli#13681) - fix(skills): honor `--dir` without agent prompt by [@happysnaker](https://github.com/happysnaker) in [#13766](cli/cli#13766) - Fix concurrent map writes in codespace port forwarding by [@williammartin](https://github.com/williammartin) in [#13313](cli/cli#13313) - Use `int64` for GitHub database IDs by [@williammartin](https://github.com/williammartin) in [#13403](cli/cli#13403) ##### 📚 Docs & Chores - Pin reusable triage workflows to a commit SHA by [@BagToad](https://github.com/BagToad) in [#13705](cli/cli#13705) - Add security disclosure guidance to `AGENTS.md` by [@BagToad](https://github.com/BagToad) in [#13720](cli/cli#13720) - Clarify `--clone` boolean flag behaviour in `gh repo fork` help by [@BagToad](https://github.com/BagToad) in [#13786](cli/cli#13786) - Fix flaky `TestHuhPrompterMultiSelectWithSearchPersistence` on slow architectures by [@pdostal](https://github.com/pdostal) in [#13675](cli/cli#13675) - docs(search): add examples for multiple qualifiers by [@happysnaker](https://github.com/happysnaker) in [#13756](cli/cli#13756) - docs: fix broken anchor link in release-process-deep-dive by [@patrickwehbe](https://github.com/patrickwehbe) in [#13688](cli/cli#13688) - docs: fix broken install command and link/grammar errors by [@patrickwehbe](https://github.com/patrickwehbe) in [#13690](cli/cli#13690) - docs: fix duplicated word in primer README by [@s3onghyun](https://github.com/s3onghyun) in [#13677](cli/cli#13677) #####

Dependencies - chore(deps): bump github.com/microsoft/dev-tunnels from 0.1.19 to 0.1.27 by [@dependabot](https://github.com/dependabot) in [#13708](cli/cli#13708) - chore(deps): bump actions/checkout from 6.0.3 to 7.0.0 by [@dependabot](https://github.com/dependabot) in [#13703](cli/cli#13703) - chore(deps): bump github.com/google/go-containerregistry from 0.21.6 to 0.21.7 by [@dependabot](https://github.com/dependabot) in [#13702](cli/cli#13702) - chore(deps): bump actions/setup-go from 6.4.0 to 6.5.0 by [@dependabot](https://github.com/dependabot) in [#13740](cli/cli#13740) - chore(deps): bump actions/attest from 4.1.0 to 4.1.1 by [@dependabot](https://github.com/dependabot) in [#13754](cli/cli#13754) - chore(deps): bump goreleaser/goreleaser-action from 7.2.2 to 7.2.3 by [@dependabot](https://github.com/dependabot) in [#13759](cli/cli#13759) - chore(deps): bump golangci/golangci-lint-action from 9.2.1 to 9.3.0 by [@dependabot](https://github.com/dependabot) in [#13779](cli/cli#13779) #### New Contributors - [@patrickwehbe](https://github.com/patrickwehbe) made their first contribution in [#13688](cli/cli#13688) - [@s3onghyun](https://github.com/s3onghyun) made their first contribution in [#13679](cli/cli#13679) - [@toller892](https://github.com/toller892) made their first contribution in [#13681](cli/cli#13681) - [@happysnaker](https://github.com/happysnaker) made their first contribution in [#13756](cli/cli#13756) **Full Changelog**: <cli/cli@v2.95.0...v2.96.0> ### [`v2.95.0`](https://github.com/cli/cli/releases/tag/v2.95.0): GitHub CLI 2.95.0 [Compare Source](cli/cli@v2.94.0...v2.95.0) #### Read repository files and directories with `gh repo read-file` and `gh repo read-dir` Two new preview commands read repository contents without cloning: ```shell # Read a single file to stdout gh repo read-file README.md --repo cli/cli # Read from a specific branch, tag, or commit gh repo read-file go.mod --ref v2.94.0 --repo cli/cli # Write a file to disk (use --clobber to overwrite) gh repo read-file README.md --output ./README.md --repo cli/cli # List the entries in a directory gh repo read-dir script --repo cli/cli ``` Both commands default to the repository's default branch, accept `--ref` to target any branch, tag, or commit, and support `--json`, `--jq`, and `--template` for scripting. This makes it easy for agents and automation to inspect a repo without a full checkout. > \[!NOTE] > `gh repo read-file` and `gh repo read-dir` are in preview and subject to change without notice. #### What's Changed ##### ✨ Features - feat: add `repo read-file` and `repo read-dir` by [@babakks](https://github.com/babakks) in [#13580](cli/cli#13580) - feat(skills): list available skills when install runs non-interactively by [@SamMorrowDrums](https://github.com/SamMorrowDrums) in [#13548](cli/cli#13548) - Support custom CLAUDE\_CONFIG\_DIR in install by [@tommaso-moro](https://github.com/tommaso-moro) in [#13523](cli/cli#13523) ##### 🐛 Fixes - fix(skills): stage updates in a temp dir and swap in-place by [@SamMorrowDrums](https://github.com/SamMorrowDrums) in [#13449](cli/cli#13449) ##### 📚 Docs & Chores - Make filtering by bot authors more discoverable by [@BagToad](https://github.com/BagToad) in [#13642](cli/cli#13642) - docs(discussion): polish help docs by [@babakks](https://github.com/babakks) in [#13632](cli/cli#13632) - Bump Go in devcontainer by [@spenserblack](https://github.com/spenserblack) in [#13674](cli/cli#13674) #####

Dependencies - chore(deps): bump golang.org/x/text from 0.37.0 to 0.38.0 by [@dependabot](https://github.com/dependabot)\[bot] in [#13640](cli/cli#13640) - chore(deps): bump charm.land/lipgloss/v2 from 2.0.3 to 2.0.4 by [@dependabot](https://github.com/dependabot)\[bot] in [#13663](cli/cli#13663) - chore(deps): bump golang.org/x/term from 0.43.0 to 0.44.0 by [@dependabot](https://github.com/dependabot)\[bot] in [#13661](cli/cli#13661) - chore(deps): bump github/codeql-action from 4.36.1 to 4.36.2 by [@dependabot](https://github.com/dependabot)\[bot] in [#13619](cli/cli#13619) - chore(deps): bump github.com/sigstore/sigstore-go from 1.1.4 to 1.2.1 by [@dependabot](https://github.com/dependabot)\[bot] in [#13662](cli/cli#13662) - chore(deps): bump golang.org/x/crypto from 0.52.0 to 0.53.0 by [@dependabot](https://github.com/dependabot)\[bot] in [#13641](cli/cli#13641) **Full Changelog**: <cli/cli@v2.94.0...v2.95.0> </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate).

fix concurrent map writes in port forward

6957ebd

Co-authored-by: Copilot <[email protected]>

github-actions Bot mentioned this pull request May 12, 2026

Testing ChatOps joshshiu/agentic-workflows-workshop#2

Open

williammartin and others added 2 commits May 12, 2026 13:30

refactor: use require/assert in port forwarder tests

8736e0b

Co-authored-by: Copilot <[email protected]>

williammartin marked this pull request as ready for review May 12, 2026 12:15

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

williammartin requested review from a team as code owners May 12, 2026 12:15

williammartin requested a review from BagToad May 12, 2026 12:15

Copilot started reviewing on behalf of williammartin May 12, 2026 12:15 View session

Copilot AI reviewed May 12, 2026

View reviewed changes

Comment thread internal/codespaces/portforwarder/port_forwarder_test.go

Comment thread internal/codespaces/portforwarder/port_forwarder.go

Comment thread internal/codespaces/portforwarder/port_forwarder.go Outdated

williammartin and others added 2 commits May 13, 2026 15:51

This comment was marked as low quality.

Sign in to view

style(codespace): remove stray blank line in import group

619ea54

Cleans up an accidental blank line splitting the stdlib imports in cmd.go. Co-authored-by: Copilot App <[email protected]>

BagToad approved these changes Jul 2, 2026

View reviewed changes

BagToad enabled auto-merge (squash) July 2, 2026 20:34

BagToad merged commit 74e7791 into trunk Jul 2, 2026
11 checks passed

BagToad deleted the williammartin/fix-concurrent-map-writes-port-forward branch July 2, 2026 20:39

BrewTestBot mentioned this pull request Jul 2, 2026

gh 2.96.0 Homebrew/homebrew-core#291107

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix concurrent map writes in codespace port forwarding#13313

Fix concurrent map writes in codespace port forwarding#13313
BagToad merged 6 commits into
trunkfrom
williammartin/fix-concurrent-map-writes-port-forward

williammartin commented Apr 29, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as low quality.

BagToad left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

williammartin commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Root cause

Fix

Test

Performance

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as low quality.

BagToad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

williammartin commented Apr 29, 2026 •

edited

Loading