Skip to content

fix(nest): per-stream session state and re-arming stream extend timer - #2335

Open
FariAmrid wants to merge 1 commit into
AlexxIT:devfrom
FariAmrid:fix/nest-extend-stream
Open

fix(nest): per-stream session state and re-arming stream extend timer#2335
FariAmrid wants to merge 1 commit into
AlexxIT:devfrom
FariAmrid:fix/nest-extend-stream

Conversation

@FariAmrid

@FariAmrid FariAmrid commented Jul 6, 2026

Copy link
Copy Markdown

Problem

Nest WebRTC/RTSP streams die shortly after start (often within a minute) and never recover. With several cameras on the same credentials it is worse: only the most recently started stream can be extended at all. This became much more visible recently as Nest media sessions got shorter-lived (see #2319), but the underlying defects are older and likely also explain long-standing reports like #2108 and #723.

Root causes (pkg/nest/api.go)

  1. All streams with the same credentials share one *API instance. NewAPI returns the cached object itself, and ExchangeSDP / GenerateRtspStream store per-session state (StreamProjectID, StreamDeviceID, StreamSessionID, StreamExpiresAt, …) on it. Every new stream overwrites the previous stream's session, so ExtendStream targets whichever camera connected last — extensions for every other camera are lost. The extendTimer != nil guard also means only one timer ever exists for all streams.

  2. The extend timer is single-shot and never re-arms. StartExtendStreamTimer schedules one extend at expiresAt − 1 min and the goroutine exits afterwards, so a session survives at most two expiry windows even when everything works. When expiresAt is less than a minute away, the computed delay is ≤ 0, the timer fires immediately, and the only extend is spent right away — the session then dies at the next expiry.

  3. Race + goroutine leak (nest: nil-pointer panic in StartExtendStreamTimer (race with StopExtendStreamTimer) crash-loops Nest WEB_RTC sources #2319). StopExtendStreamTimer sets a.extendTimer = nil while the goroutine dereferences a.extendTimer.C → nil-pointer panic. And because Timer.Stop does not close the channel, a stopped timer leaves that goroutine blocked forever — one leaked goroutine per stream start/stop cycle.

Fix

  • NewAPI returns a fresh per-caller API carrying the cached token (token caching behavior is unchanged), so each stream owns its session state and extend loop. refreshToken now uses a stored credentials key instead of the reverse cache lookup by token — that lookup also failed whenever the cached token had rotated.
  • StartExtendStreamTimer starts a loop that extends the session 30 s before every expiry (minimum 5 s delay), re-arms after each success using the refreshed expiresAt, and retries a failed extend up to 3 times, 2 s apart, before giving up.
  • The loop is controlled by a stop channel guarded by a mutex: no shared timer pointer, no nil dereference, no leaked goroutine. StopExtendStreamTimer stays idempotent.

No logging added, consistent with the logging removal in #1669.

Testing

Equivalent patch (same logic, plus debug logging) running in production since 2026-07-01 on linux/arm64 Docker with 4 WebRTC-only cameras (Nest Cam battery/wired gen 2 + Nest Doorbell, supportedProtocols=[WEB_RTC]), feeding ffmpeg recorders through the RTSP restream:

  • before: every producer died ~45–60 s after start; consumers received an initial ~2 s burst of video and then nothing (Could not find codec parameters, empty recordings);
  • after: sessions are created with a 5-minute expiry and streams survive well past the previous death point; motion-event recordings complete and save correctly.

Fixes #2319

Nest streams died shortly after start because:

- NewAPI returned the cached *API shared by every stream with the same
  credentials, so concurrent streams overwrote each other's session
  state and ExtendStream targeted the wrong session
- StartExtendStreamTimer scheduled a single extend at expiresAt-1m and
  never re-armed; with short-lived sessions the delay is <= 0, so the
  only extend fired immediately and the session died one expiry later
- StopExtendStreamTimer nils extendTimer while the goroutine
  dereferences it (panic) and the goroutine leaked after Stop

NewAPI now returns a per-caller instance carrying the cached token, and
the extend timer is a stop-channel-controlled loop that extends 30s
before each expiry and retries failed extends before giving up.

Fixes AlexxIT#2319

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@night4me

night4me commented Jul 7, 2026

Copy link
Copy Markdown

PR #2335 completely fixes the Nest session expiration issue on my system. Thank you!

I noticed one minor startup behavior.

After a cold start of Frigate, the initial ffmpeg processes sometimes start before the Nest producer has received the first video keyframe, resulting in one expected startup error:

Could not find codec parameters for stream 0 (Video: h264, none): unspecified size

The watchdog immediately restarts ffmpeg, after which both Nest cameras run normally for hours without any further issues.

So this is no longer a stability problem—only a cosmetic startup race. It might be possible to eliminate the initial log messages by delaying the RTSP restream until the producer is fully initialized, or by waiting until the first keyframe has been received before Frigate attempts to consume the stream.

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.

nest: nil-pointer panic in StartExtendStreamTimer (race with StopExtendStreamTimer) crash-loops Nest WEB_RTC sources

2 participants