Skip to content

[client] Fix nil pointer panic in ICE agent during sleep/wake cycles#5261

Merged
pappz merged 1 commit intomainfrom
fix/ice-close-nil
Feb 5, 2026
Merged

[client] Fix nil pointer panic in ICE agent during sleep/wake cycles#5261
pappz merged 1 commit intomainfrom
fix/ice-close-nil

Conversation

@pappz
Copy link
Copy Markdown
Collaborator

@pappz pappz commented Feb 5, 2026

Add defensive nil checks in ThreadSafeAgent.Close() to prevent panic when agent field is nil. This can occur during Windows suspend/resume when network interfaces are disrupted or the pion/ice library returns nil without error.

Also capture agent pointer in local variable before goroutine execution to prevent race conditions.

Fixes service crashes on laptop wake-up.

Describe your changes

Issue ticket number and link

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced ICE (Interactive Connectivity Establishment) agent initialization with better error detection and reporting
    • Improved ICE connection cleanup operations with enhanced error handling and logging

Add defensive nil checks in ThreadSafeAgent.Close() to prevent panic
when agent field is nil. This can occur during Windows suspend/resume
when network interfaces are disrupted or the pion/ice library returns
nil without error.

Also capture agent pointer in local variable before goroutine execution
to prevent race conditions.

Fixes service crashes on laptop wake-up.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 5, 2026

📝 Walkthrough

Walkthrough

Added nil-safety checks to ICE agent initialization and closing operations. The NewAgent function now validates that the created agent is non-nil before proceeding, and the Close implementation guards against nil agents while preserving timeout-based cleanup behavior. Worker code also now checks before closing agents and logs errors.

Changes

Cohort / File(s) Summary
ICE Agent Lifecycle Management
client/internal/peer/ice/agent.go
Added nil-check after ice.NewAgent with formatted error return; modified Close to guard against nil agent, log warnings if nil, and preserve timeout-based cleanup via conditional goroutine execution.
Worker ICE Agent Cleanup
client/internal/peer/worker_ice.go
Added nil-check before calling Close() on ICE agent and implemented per-call error logging for close failures.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers

  • lixmal

Poem

🐰 A tale of pointers, once so wild,
Now checked for nil, both stern and mild,
With guards and timeouts standing tall,
Safe Close calls prevail for all!

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description covers the core issue, provides context about the Windows suspend/resume scenario, and explains the fix. However, the 'Describe your changes' section remains empty, which is a required template section. Fill in the 'Describe your changes' section with details about the defensive nil checks and local variable capture implementation to complete the template.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the main change: adding nil pointer panic prevention in ICE agent during sleep/wake cycles, directly matching the changeset's defensive nil checks.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/ice-close-nil

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Feb 5, 2026

@pappz pappz merged commit d2f9653 into main Feb 5, 2026
38 of 40 checks passed
@pappz pappz deleted the fix/ice-close-nil branch February 5, 2026 11:06
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.

invalid memory address or nil pointer dereference on windows arm64 when sleeping laptop

2 participants