Repository navigation
fix(hub): don't read the SSH client after it is closed - #2277
Merged
Merged
Conversation
createSessionWithTimeout checked sys.client for nil and then dereferenced it again inside the goroutine that calls NewSession. update() runs the SMART fetch in its own goroutine, so closeSSHConnection can clear the field between those two reads and the goroutine dereferences a nil client, panicking the whole hub process. Make client an atomic.Pointer, load it once before starting the goroutine, and clear it with Swap so a concurrent close cannot be observed mid-session-creation. NewSession on an already-closed client returns an error, which the existing retry path already handles. Closes henrygd#2157
henrygd
approved these changes
Aug 30, 2026
henrygd
left a comment
Owner
There was a problem hiding this comment.
Looks like a good solution, thanks for your work 👍
JuliusFreudenberger
pushed a commit
to JuliusFreudenberger/nix-config
that referenced
this pull request
Sep 3, 2026
This PR contains the following updates: | Package | Update | Change | |---|---|---| | [henrygd/beszel](https://github.com/henrygd/beszel) | minor | `0.18.8` → `0.19.0` | --- >⚠️ **Warning** > > Some dependencies could not be looked up. Check the [Dependency Dashboard](issues/12) for more information. --- ### Release Notes <details> <summary>henrygd/beszel (henrygd/beszel)</summary> ### [`v0.19.0`](https://github.com/henrygd/beszel/releases/tag/v0.19.0) [Compare Source](henrygd/beszel@v0.18.8...v0.19.0) :warning: **Potential breaking change:** Agents now verify HTTPS certificates. If an agent connects to a hub using a self-signed or otherwise untrusted certificate, configure `CA_CERT_FILE` with the appropriate CA certificate or the connection will be rejected. #### What's Changed - Add ZFS pool and dataset monitoring ([#​2209](henrygd/beszel#2209)) - Add container health alerts with log excerpts in notifications ([#​2225](henrygd/beszel#2225)) - Add alerts for failed systemd services ([#​2173](henrygd/beszel#2173)) - Add alerts for CPU I/O wait and steal time ([#​2249](henrygd/beszel#2249)) - Add cumulative disk read and write totals to the Disk I/O sheet ([#​2179](henrygd/beszel#2179)) - Add Greek and Uzbek translations ([#​2034](henrygd/beszel#2034)) - Add custom root disk names and sort disks alphabetically ([#​2006](henrygd/beszel#2006)) - Improve alert security by enforcing system ownership and blocking additional internal notification URL ranges - Improve ATA SMART warnings for critical attributes ([#​2275](henrygd/beszel#2275)) - Fix a hub crash caused by concurrent SSH client shutdown ([#​2277](henrygd/beszel#2277)) - Fix host CPU and memory totals when Docker runs in a VM on macOS or Windows ([#​2272](henrygd/beszel#2272)) - Fix multi-line `TOKEN_FILE` loading ([#​2276](henrygd/beszel#2276)) - Fix QNAP MD RAID arrays being incorrectly reported as failed ([#​2065](henrygd/beszel#2065)) - Fix Intel GPU power data disappearing between samples ([#​2256](henrygd/beszel#2256)) - Fix swap usage calculations to include cached swap ([#​2267](henrygd/beszel#2267)) - Fix agent payload failures caused by invalid UTF-8 in battery names ([#​2241](henrygd/beszel#2241)) - Fix uptime reporting inside Linux containers ([#​2180](henrygd/beszel#2180)) - Fix fan discovery on legacy hwmon devices ([#​2238](henrygd/beszel#2238)) - Fix CPU model detection on MIPS systems ([#​2138](henrygd/beszel#2138)) - Fix machine ID generation on FreeBSD and pfSense installations ([#​2274](henrygd/beszel#2274)) - Fix agent environment variable updates during reinstall ([#​2107](henrygd/beszel#2107)) - Fix the install script reporting the wrong listen address after upgrades ([#​2243](henrygd/beszel#2243)) - Fix load average precision on macOS, BSD, and Windows ([#​2245](henrygd/beszel#2245)) - Update the Go version and Go dependencies #### New Contributors - [@​IlyaMur](https://github.com/IlyaMur) made their first contribution in [#​2238](henrygd/beszel#2238) - [@​alecrubin](https://github.com/alecrubin) made their first contribution in [#​2180](henrygd/beszel#2180) - [@​toomore](https://github.com/toomore) made their first contribution in [#​2241](henrygd/beszel#2241) - [@​mews-se](https://github.com/mews-se) made their first contribution in [#​2243](henrygd/beszel#2243) - [@​justadityaraj](https://github.com/justadityaraj) made their first contribution in [#​2245](henrygd/beszel#2245) - [@​wassupluke](https://github.com/wassupluke) made their first contribution in [#​2256](henrygd/beszel#2256) - [@​ErkinjonYusupov](https://github.com/ErkinjonYusupov) made their first contribution in [#​2034](henrygd/beszel#2034) - [@​snh](https://github.com/snh) made their first contribution in [#​2279](henrygd/beszel#2279) - [@​spatiumstas](https://github.com/spatiumstas) made their first contribution in [#​2249](henrygd/beszel#2249) - [@​martinstenrose](https://github.com/martinstenrose) made their first contribution in [#​2173](henrygd/beszel#2173) - [@​mleczakm](https://github.com/mleczakm) made their first contribution in [#​2225](henrygd/beszel#2225) - [@​santhiprakash](https://github.com/santhiprakash) made their first contribution in [#​2269](henrygd/beszel#2269) **Full Changelog**: <henrygd/beszel@v0.18.8...v0.19.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 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 [Mend Renovate](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xNTAuMSIsInVwZGF0ZWRJblZlciI6IjQzLjE1MC4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Reviewed-on: https://git.jfreudenberger.de/JuliusFreudenberger/nix-config/pulls/43
xiaoran7
pushed a commit
to xiaoran7/beszel
that referenced
this pull request
Oct 17, 2026
createSessionWithTimeout checked sys.client for nil and then dereferenced it again inside the goroutine that calls NewSession. update() runs the SMART fetch in its own goroutine, so closeSSHConnection can clear the field between those two reads and the goroutine dereferences a nil client, panicking the whole hub process. Make client an atomic.Pointer, load it once before starting the goroutine, and clear it with Swap so a concurrent close cannot be observed mid-session-creation. NewSession on an already-closed client returns an error, which the existing retry path already handles. Closes henrygd#2157
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📃 Description
Closes #2157.
createSessionWithTimeoutcheckssys.clientfor nil, then dereferences it a second time inside the goroutine that callsNewSession. Those two reads are not the same read.update()runs the SMART fetch in its own goroutine (system.go:166), so the updater goroutine can callcloseSSHConnectionand clear the field in between, and the session goroutine dereferences a nil client. That panic is not recovered anywhere, so it takes down the whole hub process.The client field is now an
atomic.Pointer[ssh.Client], loaded once before the goroutine starts and cleared withSwap, matching theatomic.Boolfields already on the struct.NewSessionon an already-closed client returns an error, whichrunSSHOperationalready retries.createSSHClientstill stores the dial result unconditionally, so a failed dial clears the field exactly as before.🪵 Changelog
🔧 Fixed
Verification
TestCreateSessionDuringCloseruns session creation againstcloseSSHConnectionand reproduces the reported crash onmainwith the same frames as the issue:It panicked on 3 of 3 runs before the change and passes after it.
go test -tags testing ./internal/hub/...:config,expirymap,heartbeat,systems,transportandwsall pass.internal/hubitself does not build on this Windows host, before or after the change, becauseagent/sensors_windows.goembeds a LibreHardwareMonitor binary that is not checked in.go vet -tags testing ./internal/hub/systems/...andgofmt -lclean.-racehere (no cgo toolchain on this machine), which is part of why the field moved to an atomic rather than a plain captured pointer.