[PR #5222] [MERGED] server/group: refactor with shared abstractions and fix concurrency issues #5187

New issue

Closed

opened 2026-05-05 14:56:20 -06:00 by gitea-mirror · 0 comments

gitea-mirror commented

2026-05-05 14:56:20 -06:00

Owner

📋 Pull Request Information

Original PR: https://github.com/fatedier/frp/pull/5222
Author: @fatedier
Created: 3/8/2026
Status: ✅ Merged
Merged: 3/8/2026
Merged by: @fatedier

Base: dev ← Head: new

📝 Commits (3)

0125ca9 server/group: refactor group package with shared abstractions and fix concurrency issues
2de5655 server/group: replace tautological assertion with require.NotPanics
9eafcc8 server/group: remove blank line between doc comment and type declaration

📊 Changes

12 files changed (+720 additions, -436 deletions)

View changed files

➕ server/group/base.go (+77 -0)
➕ server/group/base_test.go (+169 -0)
📝 server/group/group.go (+2 -0)
📝 server/group/http.go (+26 -33)
📝 server/group/https.go (+28 -123)
➕ server/group/listener.go (+49 -0)
➕ server/group/listener_test.go (+68 -0)
➕ server/group/registry.go (+59 -0)
➕ server/group/registry_test.go (+102 -0)
📝 server/group/tcp.go (+37 -138)
📝 server/group/tcpmux.go (+41 -142)
📝 test/e2e/v1/features/group.go (+62 -0)

📄 Description

Summary

Extract common patterns into reusable components: generic groupRegistry[G], shared baseGroup for listener-based groups, unified Listener type
Fix concurrency issues: stale-pointer race (isCurrent + retry loop), worker generation safety, connection leak on panic, ABBA deadlock in HTTP UnRegister, round-robin overflow
Add 17 unit tests for registry, listener, and baseGroup
Add TCPMux group load balancing e2e test (previously zero coverage)

Test plan

go test -race ./server/group — 17 tests pass
golangci-lint run — 0 issues
make e2e — 225/225 pass (2 skipped: IPv6/QUIC)
Codex review — no actionable issues

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/fatedier/frp/pull/5222 **Author:** [@fatedier](https://github.com/fatedier) **Created:** 3/8/2026 **Status:** ✅ Merged **Merged:** 3/8/2026 **Merged by:** [@fatedier](https://github.com/fatedier) **Base:** `dev` ← **Head:** `new` --- ### 📝 Commits (3) - [`0125ca9`](https://github.com/fatedier/frp/commit/0125ca94375bcdd7b01265b0de183042044f7354) server/group: refactor group package with shared abstractions and fix concurrency issues - [`2de5655`](https://github.com/fatedier/frp/commit/2de56556d919c27c30052d2e1e4b29a743de50d8) server/group: replace tautological assertion with require.NotPanics - [`9eafcc8`](https://github.com/fatedier/frp/commit/9eafcc8a9526ffe0f91f9f6c6f687f92e2db4fce) server/group: remove blank line between doc comment and type declaration ### 📊 Changes **12 files changed** (+720 additions, -436 deletions) <details> <summary>View changed files</summary> ➕ `server/group/base.go` (+77 -0) ➕ `server/group/base_test.go` (+169 -0) 📝 `server/group/group.go` (+2 -0) 📝 `server/group/http.go` (+26 -33) 📝 `server/group/https.go` (+28 -123) ➕ `server/group/listener.go` (+49 -0) ➕ `server/group/listener_test.go` (+68 -0) ➕ `server/group/registry.go` (+59 -0) ➕ `server/group/registry_test.go` (+102 -0) 📝 `server/group/tcp.go` (+37 -138) 📝 `server/group/tcpmux.go` (+41 -142) 📝 `test/e2e/v1/features/group.go` (+62 -0) </details> ### 📄 Description ## Summary - Extract common patterns into reusable components: generic `groupRegistry[G]`, shared `baseGroup` for listener-based groups, unified `Listener` type - Fix concurrency issues: stale-pointer race (isCurrent + retry loop), worker generation safety, connection leak on panic, ABBA deadlock in HTTP UnRegister, round-robin overflow - Add 17 unit tests for registry, listener, and baseGroup - Add TCPMux group load balancing e2e test (previously zero coverage) ## Test plan - [x] `go test -race ./server/group` — 17 tests pass - [x] `golangci-lint run` — 0 issues - [x] `make e2e` — 225/225 pass (2 skipped: IPv6/QUIC) - [x] Codex review — no actionable issues --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>