Skip to content

Let the poll server restart after CleanupSRT - #84

Merged
jeoliva merged 1 commit into
masterfrom
fix/pollserver-restart
Jul 29, 2026
Merged

Let the poll server restart after CleanupSRT#84
jeoliva merged 1 commit into
masterfrom
fix/pollserver-restart

Conversation

@jeoliva

@jeoliva jeoliva commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Fixes a regression I introduced in the previous commit, caught by testing a start/stop/start cycle that nothing in the suite covered.

The bug

CleanupSRT stops the poll server and releases its epoll. But the poll server was created behind a sync.Once, so it could never be replaced. The next non-blocking socket tried to register against the released epoll:

panic: ERROR ADDING FD TO EPOLL
  pollserver.go:82 -> poll.go:109 -> srtgo.go:122

This sequence worked at 9576f32 and panicked after:

InitSRT(); sock := NewSrtSocket(...); sock.Close()
CleanupSRT()
InitSRT(); sock2 := NewSrtSocket(...)   // panic

Start/stop/start is a reasonable thing for a consumer to do — and the whole point of CleanupSRT being exported.

The fix

Replace the sync.Once with a nil-checked, mutex-guarded pointer. pollServerShutdown clears it; the next socket constructs a fresh poll server with a fresh epoll. shutdown() itself stays idempotent via its own stopOnce.

Verification

  • The new TestPollServerRestartsAfterCleanup panics without this change and passes with it.
  • The exit-teardown fix is unaffected: full suite under -race, no segfaults, 10/10 on macOS arm64 and 10/10 on Linux amd64.

CI did not catch this because no test exercised a restart. That gap is now closed.

🤖 Generated with Claude Code

Regression from the shutdown support added alongside CI. The poll server was
created behind a sync.Once, so once CleanupSRT had stopped it and released
its epoll, it could never be replaced. The next non-blocking socket then
tried to register against the released epoll:

    panic: ERROR ADDING FD TO EPOLL
      pollserver.go:82 -> poll.go:109 -> srtgo.go:122

So InitSRT -> sockets -> CleanupSRT -> InitSRT -> socket, which worked at
9576f32, panicked afterwards. Start/stop/start is a reasonable thing for a
consumer to do, and nothing in the suite covered it.

Replace the sync.Once with a nil-checked, mutex-guarded pointer: shutdown
clears it, and the next socket constructs a fresh poll server with a fresh
epoll. Shutdown itself stays idempotent.

Adds the missing regression test, which panics without this change.

Verified: restart test passes; full suite under -race stays clean, 10/10 on
macOS arm64 and 10/10 on Linux amd64 with no segfaults, so the exit-teardown
fix is unaffected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jeoliva
jeoliva merged commit 49fd956 into master Jul 29, 2026
4 checks passed
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.

1 participant