fix(morpc): join accepted server connections on close

[ck89119]

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

issue #26550

What this PR does / why we need it:

MORPC server shutdown previously stopped the goetty application and returned after disconnecting active sessions, but goetty did not join the admitted connection handlers. Under CI load, leak detection could therefore run while doConnection, server write-loop, and peer backend goroutines were still unwinding.

This change:

• tracks every connection admitted by the goetty application;
• runs the connection read loop through an MORPC-owned handler so its completion is observable;
• makes successful server shutdown wait until all admitted handlers have returned;
• preserves exactly-once completion when a session is rejected before its handler starts; and
• adds a deterministic regression test proving that Server.Close cannot return before an accepted connection completes cleanup.

Validation:

• mo-cgo-test -race -count=100 -run '^TestServerCloseWaitsForAcceptedConnections$' ./pkg/common/morpc
• mo-cgo-test -race -count=1 ./pkg/common/morpc
• mo-cgo-test -count=50 -run '^TestStatusInRollingRestartCN$' ./pkg/lockservice
• mo-cgo-test -race -count=13 -run '^TestStatusInRollingRestartCN$' ./pkg/lockservice
• mo-cgo-test -race -count=1 ./pkg/lockservice
• go build ./pkg/common/morpc/... ./pkg/lockservice/...
• go vet ./pkg/common/morpc/... ./pkg/lockservice/...

After merging the latest main, both focused race regressions were rerun successfully.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[XuPeng-SH]

Deep review finding — P1 / merge blocker

The connection tracker is admitted too late to provide the shutdown guarantee claimed by this PR. goetty first accepts the connection, calls addSession(rs), and launches the connection handler with go func() { handle(rs) }. However, MORPC only calls connections.begin() after that goroutine is scheduled and enters handleConnection.

This leaves a valid shutdown interleaving:

1. The connection is accepted and added to goetty's session set.
2. The handler goroutine is created but has not run yet.
3. server.Close() calls application.Stop().
4. sealAndWait() observes active == 0 and returns.
5. The already-created handler goroutine runs afterward; begin() sees the tracker sealed and returns.

So Close joins callbacks that have already started, not all connections already admitted by the server. The goetty handler goroutine from #26550 can therefore still outlive Close. Also, finish() runs inside the callback, before the outer goetty goroutine finishes its deferred session cleanup, so the tracker does not join the complete connection goroutine even on the started path.

The new test does not cover this boundary because it manually calls tracker.begin() before invoking Close(). I reproduced the missing state with a deterministic gated-handler test; the current implementation fails immediately because Close() returns before an already accepted handler is first scheduled. The existing test passes under -race -count=20, and the full morpc race suite passes, confirming that CI is exercising only the already-started path.

Please move the lifecycle accounting to the actual goetty admission/ownership boundary:

• reserve the connection before launching its goroutine;
• release it with a defer at the outermost goroutine boundary;
• in Stop(), stop and join accept loops first so no more reservations can occur;
• disconnect sessions to release blocked reads;
• then wait for all admitted connection goroutines to exit.

Please also add a deterministic regression test for the accepted-but-handler-not-yet-started state. This fixes the ownership protocol rather than relying on scheduling or a scenario-specific delay.

[XuPeng-SH]

Requesting changes for the P1 shutdown-ownership gap documented in the prior comment.

connections.begin() executes only after the goetty handler goroutine is scheduled, while connection admission happens before that goroutine is launched. Close() can therefore seal an empty tracker and return while an already accepted handler is still queued, leaving the original goroutine-leak failure possible.

Please move accounting to the goetty admission/outer-goroutine boundary and add a deterministic accepted-but-not-yet-started regression test.