conn: return a close function from NewConfig and remove Config.Close

[meling]

Closes #314

Problem

All configurations derived from one NewConfig call share one connection pool. Config.Close closed that whole pool from any configuration in it, so cfg.Remove(3).Close() also closed node 3. #351 documented this rule and fixed the leak and the data race around it, but the rule still surprises users.

A Config is a plain []*Node. It cannot record whether it owns the pool, so no rule for Config.Close can avoid the surprise. Reference counting on Node does not work either; see the decision comment on #314.

Change

NewConfig returns a close function, and Config has no Close method:

cfg, closeFn, err := gorums.NewConfig(gorums.WithNodeList(addrs), dialOpts...)
if err != nil {
	return err
}
defer closeFn()

• Only the creator holds the close function. Derived configurations (Extend, Remove, Sort, WithoutErrors, slicing) are views and cannot close the pool.
• The close function closes every node in the pool, including nodes that Extend added. It is idempotent and safe for concurrent use.
• It returns no error. The only error the close path can produce is gRPC's deprecated ErrClientConnClosing, for a connection that is already closed, and Gorums closes each connection once. The manager logs a node's close error if a logger is set.
• On error, NewConfig returns a nil configuration and a nil close function.
• Server keeps the close function for its WithPeers configuration and calls it in Stop and GracefulStop. A handler can no longer close the server's peer pool through ServerContext.PeerConfig().
• gorumstest.Closer existed only to check the close error, so it is removed. Tests pass the close function to t.Cleanup directly.
• The internals from conn: reject new nodes after Close and lock the node list in Close #351 are unchanged: the closed flag, the locking, and the addNode and Extend checks.

This is a breaking API change. No release has been made since v0.11.0.

Testing

• TestConfigClose is rewritten around the close function:
  • ClosesExtendedNodes: nodes that Extend added are closed too.
  • Idempotent: concurrent calls all return after the nodes are closed.
  • ExtendAfterClose: new node, existing node, and nil source all fail.
  • NewConfigError: a failing NewConfig returns nil, nil and leaks nothing (goleak). Removing the cleanup on the error path makes this subtest fail.
  • ConcurrentExtend: unchanged.
• TestNewConfig and TestNewConfigWithBackChannel also check that both results are nil on error.
• go test ./... passes in the root, benchkit, and examples modules.
• go test -race -count=5 . ./internal/conn/ ./gorumstest/ passes.
• go test -tags=integration ./... passes.
• make modernize and make goplscheck are clean for the changed files.

[Copilot]

🟡 Changes recommended

Two guide examples omit cleanup, and the documented error-result assertion is missing from the back-channel test.

2 open findings

• Missing assertion that both results are nil on validation error · New
• Example never invokes pool close function · New

What changed in this PR

Moves connection-pool ownership from Config views to a close function returned by NewConfig.

Changes:

• Removes Config.Close and updates all callers.
• Makes pool shutdown idempotent, concurrent-safe, and server-owned for peers.
• Updates lifecycle tests and documentation.

File	Description
config.go	Exposes the new NewConfig signature.
config_test.go	Tests pool closure and error behavior.
internal/​conn/​config.go	Returns the pool close function.
internal/​conn/​config_test.go	Removes obsolete close tests.
internal/​conn/​outbound_manager.go	Logs close errors and returns no error.
server.go	Stores and invokes peer-pool cleanup.
server_test.go	Migrates test cleanup calls.
server_internal_test.go	Migrates internal peer helpers.
gorumstest/​gorumstest.go	Removes Closer and registers close functions.
gorumstest/​servers.go	Updates example usage.
examples/​storage/​client.go	Uses the returned close function.
benchkit/​benchmark/​target.go	Returns the new cleanup function.
benchkit/​benchmark/​benchmark_test.go	Migrates benchmark test cleanup.
doc/​user-guide.md	Documents the ownership model.
doc/​dev-guide.md	Updates internal lifecycle documentation.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.