vole
Clone
Read only, no credentials:
https://rodent.software/git/vole.git
Write over SSH with a registered key:
git clone ssh://git@0.0.0.0:2222/vole.git
A free and open-source forge that lives in your Git repo.
Make a closed SSH server stop before it returns
786d7ce1a17ddb2224cefedbe73564cc8165ca32 · Keenan Nemetz · 2d ago · parent f642e0aa
Make a closed SSH server stop before it returns CI caught the new concurrency test failing on a temporary directory that was not empty: a connection handler was still finishing a disconnect when the test harness removed the directory it had handed the server. The test was the trigger, not the cause — Close() only closed the listener and left every handler and session goroutine running. Close now ends the connections it is serving and waits for their goroutines, so "closed" means nothing this server started is still writing. The wait is bounded (10s) and reported, because a git process stuck on a dead filesystem must not hold up a shutdown. A parked lease answers on Close as well, rather than making the wait sit out its 25-second window. Refs: 01M6SQ7WC68KGP57MN1DS3DFPQ (#220)
server/ssh/server.go +91−4
| @@ -128,6 +128,16 @@ type Server struct { | |||
| 128 | 128 | // also the wait. | |
| 129 | 129 | conns chan struct{} | |
| 130 | 130 | gitSlots chan struct{} | |
| 131 | // done closes when the server is closing, so a lease parked on the queue answers | ||
| 132 | // instead of sitting out its wait. | ||
| 133 | done chan struct{} | ||
| 134 | // live is the connections being served, and handlers waits for their goroutines | ||
| 135 | // and session goroutines. Close ends them and returns only when nothing this | ||
| 136 | // server started is still running: a closed server must not be writing into a | ||
| 137 | // directory somebody is about to remove. | ||
| 138 | live map[net.Conn]struct{} | ||
| 139 | handlers sync.WaitGroup | ||
| 140 | closing bool | ||
| 131 | 141 | // hostKey is this instance's public host key as one authorized_keys line. It | |
| 132 | 142 | // travels with a lease so a job that acts as a bot can write the known_hosts | |
| 133 | 143 | // entry it needs without trusting whatever answers a scan. | |
| @@ -183,6 +193,8 @@ func New(cfg Config) (*Server, error) { | |||
| 183 | 193 | cfg: cfg, | |
| 184 | 194 | conns: make(chan struct{}, cfg.maxConnections()), | |
| 185 | 195 | gitSlots: make(chan struct{}, cfg.maxGitProcesses()), | |
| 196 | done: make(chan struct{}), | ||
| 197 | live: make(map[net.Conn]struct{}), | ||
| 186 | 198 | hostKey: strings.TrimSpace(string(xssh.MarshalAuthorizedKey(signer.PublicKey()))), | |
| 187 | 199 | } | |
| 188 | 200 | s.sshCfg = &xssh.ServerConfig{ | |
| @@ -249,8 +261,16 @@ func (s *Server) Serve(ln net.Listener) error { | |||
| 249 | 261 | conn.Close() | |
| 250 | 262 | continue | |
| 251 | 263 | } | |
| 264 | if !s.track(conn) { | ||
| 265 | // Close won the race for this connection: it is not being served. | ||
| 266 | <-s.conns | ||
| 267 | conn.Close() | ||
| 268 | continue | ||
| 269 | } | ||
| 252 | 270 | go func(c net.Conn) { | |
| 253 | 271 | defer func() { <-s.conns }() | |
| 272 | defer s.handlers.Done() | ||
| 273 | defer s.forget(c) | ||
| 254 | 274 | s.handleConn(c) | |
| 255 | 275 | }(conn) | |
| 256 | 276 | } | |
| @@ -293,13 +313,74 @@ func (s *Server) acquireGitProcess(wait time.Duration) (func(), bool) { | |||
| 293 | 313 | } | |
| 294 | 314 | } | |
| 295 | 315 | ||
| 296 | func (s *Server) Close() error { | ||
| 316 | // closeGrace bounds how long Close waits for connection handlers. A handler that is | ||
| 317 | // still running after this is reported rather than waited for. | ||
| 318 | const closeGrace = 10 * time.Second | ||
| 319 | |||
| 320 | // track records a connection to serve, or reports false when the server is already | ||
| 321 | // closing. Adding to the WaitGroup under the same lock that sets closing is what | ||
| 322 | // makes Close's wait well defined: every handler is either registered before the | ||
| 323 | // wait starts or never started. | ||
| 324 | func (s *Server) track(conn net.Conn) bool { | ||
| 297 | 325 | s.mu.Lock() | |
| 298 | 326 | defer s.mu.Unlock() | |
| 299 | if s.ln != nil { | ||
| 300 | return s.ln.Close() | ||
| 327 | if s.closing { | ||
| 328 | return false | ||
| 329 | } | ||
| 330 | s.live[conn] = struct{}{} | ||
| 331 | s.handlers.Add(1) | ||
| 332 | return true | ||
| 333 | } | ||
| 334 | |||
| 335 | func (s *Server) forget(conn net.Conn) { | ||
| 336 | s.mu.Lock() | ||
| 337 | delete(s.live, conn) | ||
| 338 | s.mu.Unlock() | ||
| 339 | } | ||
| 340 | |||
| 341 | // Close stops the listener, ends the connections being served, and waits for their | ||
| 342 | // goroutines. Waiting is the point: a server that has been closed has no handler | ||
| 343 | // left to write into its data directory, which is what a caller shutting down — or a | ||
| 344 | // test removing the temporary directory it handed the server — has to be able to | ||
| 345 | // assume. | ||
| 346 | func (s *Server) Close() error { | ||
| 347 | s.mu.Lock() | ||
| 348 | if s.closing { | ||
| 349 | // Closing a closed server is a no-op rather than an error: every caller is a | ||
| 350 | // shutdown path, and none of them can do anything about it. | ||
| 351 | s.mu.Unlock() | ||
| 352 | return nil | ||
| 353 | } | ||
| 354 | s.closing = true | ||
| 355 | ln := s.ln | ||
| 356 | live := make([]net.Conn, 0, len(s.live)) | ||
| 357 | for c := range s.live { | ||
| 358 | live = append(live, c) | ||
| 359 | } | ||
| 360 | s.mu.Unlock() | ||
| 361 | |||
| 362 | close(s.done) | ||
| 363 | var err error | ||
| 364 | if ln != nil { | ||
| 365 | err = ln.Close() | ||
| 366 | } | ||
| 367 | for _, c := range live { | ||
| 368 | _ = c.Close() | ||
| 369 | } | ||
| 370 | // The wait is bounded because a git process stuck on a dead filesystem must not | ||
| 371 | // hold up a shutdown forever. Everything a healthy handler does when its | ||
| 372 | // connection ends — noticing the close, writing one last row — takes far less. | ||
| 373 | finished := make(chan struct{}) | ||
| 374 | go func() { | ||
| 375 | s.handlers.Wait() | ||
| 376 | close(finished) | ||
| 377 | }() | ||
| 378 | select { | ||
| 379 | case <-finished: | ||
| 380 | case <-time.After(closeGrace): | ||
| 381 | s.cfg.Log.Warn("ssh: connections did not finish closing", "grace", closeGrace) | ||
| 301 | 382 | } | |
| 302 | return nil | ||
| 383 | return err | ||
| 303 | 384 | } | |
| 304 | 385 | ||
| 305 | 386 | func (s *Server) handleConn(c net.Conn) { | |
| @@ -330,6 +411,9 @@ func (s *Server) handleConn(c net.Conn) { | |||
| 330 | 411 | // One token per open session channel. A flood is rejected at the channel layer | |
| 331 | 412 | // rather than spawning a goroutine per request. | |
| 332 | 413 | slots := make(chan struct{}, maxChannelsPerConnection) | |
| 414 | // Closed waits for the session goroutines, so a connection handler is not done | ||
| 415 | // until the work it started is: Close relies on that. | ||
| 416 | var sessions sync.WaitGroup | ||
| 333 | 417 | for newCh := range chans { | |
| 334 | 418 | if newCh.ChannelType() != "session" { | |
| 335 | 419 | newCh.Reject(xssh.UnknownChannelType, "only session channels are supported") | |
| @@ -347,11 +431,14 @@ func (s *Server) handleConn(c net.Conn) { | |||
| 347 | 431 | <-slots | |
| 348 | 432 | continue | |
| 349 | 433 | } | |
| 434 | sessions.Add(1) | ||
| 350 | 435 | go func(ch xssh.Channel, reqs <-chan *xssh.Request) { | |
| 436 | defer sessions.Done() | ||
| 351 | 437 | defer func() { <-slots }() | |
| 352 | 438 | s.handleSession(ch, reqs, user, fp, sconn.RemoteAddr().String()) | |
| 353 | 439 | }(ch, chReqs) | |
| 354 | 440 | } | |
| 441 | sessions.Wait() | ||
| 355 | 442 | } | |
| 356 | 443 | ||
| 357 | 444 | func (s *Server) handleSession(ch xssh.Channel, reqs <-chan *xssh.Request, user, fp, remote string) { | |