From b294905857830d12899e007bc07da7427727fab4 Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Wed, 30 Nov 2022 12:32:27 -0500 Subject: [PATCH] Fixes: d827725a527e ("fix(server): git daemon") --- git/config.go | 7 +++ server/git/daemon/daemon.go | 101 ++++++++++++++++++++---------------- server/git/error.go | 4 +- 3 files changed, 64 insertions(+), 48 deletions(-) diff --git a/git/config.go b/git/config.go index f4fec12889389c876af4b7a54158b09cdde51820..4e9af6ed500302e8d1e7e7afeabf13bbba4bde48 100644 --- a/git/config.go +++ b/git/config.go @@ -5,6 +5,7 @@ type ConfigOptions struct { File string All bool Add bool + CommandOptions } // Config gets a git configuration. @@ -20,6 +21,9 @@ func Config(key string, opts ...ConfigOptions) (string, error) { if opt.All { cmd.AddArgs("--get-all") } + for _, a := range opt.Args { + cmd.AddArgs(a) + } cmd.AddArgs(key) bts, err := cmd.Run() if err != nil { @@ -38,6 +42,9 @@ func SetConfig(key string, value string, opts ...ConfigOptions) error { if opt.File != "" { cmd.AddArgs("--file", opt.File) } + for _, a := range opt.Args { + cmd.AddArgs(a) + } cmd.AddArgs(key, value) _, err := cmd.Run() return err diff --git a/server/git/daemon/daemon.go b/server/git/daemon/daemon.go index 75d285781dee2941ce0cd91cf813b410bbb79e78..40d4596c4fc57c78d3f07b21bd19ccd5a37bf091 100644 --- a/server/git/daemon/daemon.go +++ b/server/git/daemon/daemon.go @@ -27,9 +27,11 @@ type Daemon struct { addr string finished chan struct{} conns map[net.Conn]struct{} + count int cfg *config.Config wg sync.WaitGroup once sync.Once + mtx sync.RWMutex } // NewDaemon returns a new Git daemon. @@ -37,7 +39,7 @@ func NewDaemon(cfg *config.Config) (*Daemon, error) { addr := fmt.Sprintf("%s:%d", cfg.Host, cfg.Git.Port) d := &Daemon{ addr: addr, - finished: make(chan struct{}), + finished: make(chan struct{}, 1), cfg: cfg, conns: make(map[net.Conn]struct{}), } @@ -52,50 +54,59 @@ func NewDaemon(cfg *config.Config) (*Daemon, error) { // Start starts the Git TCP daemon. func (d *Daemon) Start() error { defer d.listener.Close() - // set up channel on which to send accepted connections - listen := make(chan net.Conn, d.cfg.Git.MaxConnections) - d.wg.Add(1) - go d.acceptConnection(d.listener, listen) - - // loop work cycle with accept connections or interrupt - // by system signal - for { - select { - case conn := <-listen: - d.wg.Add(1) - go func() { - d.handleClient(conn) - d.wg.Done() - }() - case <-d.finished: - return ErrServerClosed - } - } -} - -func fatal(c net.Conn, err error) { - git.WritePktline(c, err) - if err := c.Close(); err != nil { - log.Printf("git: error closing connection: %v", err) - } -} -// acceptConnection accepts connections on the listener. -func (d *Daemon) acceptConnection(listener net.Listener, listen chan<- net.Conn) { + d.wg.Add(1) defer d.wg.Done() + + var tempDelay time.Duration for { - conn, err := listener.Accept() + conn, err := d.listener.Accept() if err != nil { select { case <-d.finished: - log.Printf("git: %s", ErrServerClosed) - return + return ErrServerClosed default: log.Printf("git: error accepting connection: %v", err) + } + if ne, ok := err.(net.Error); ok && ne.Temporary() { + if tempDelay == 0 { + tempDelay = 5 * time.Millisecond + } else { + tempDelay *= 2 + } + if max := 1 * time.Second; tempDelay > max { + tempDelay = max + } + time.Sleep(tempDelay) continue } + return err + } + + // Close connection if there are too many open connections. + var count int + d.mtx.RLock() + count = d.count + d.mtx.RUnlock() + log.Printf("count: %d", count) + if count+1 >= d.cfg.Git.MaxConnections { + log.Printf("git: max connections reached, closing %s", conn.RemoteAddr()) + fatal(conn, git.ErrMaxConnections) + continue } - listen <- conn + + d.wg.Add(1) + go func() { + d.handleClient(conn) + d.wg.Done() + }() + } +} + +func fatal(c net.Conn, err error) { + git.WritePktline(c, err) + if err := c.Close(); err != nil { + log.Printf("git: error closing connection: %v", err) } } @@ -112,17 +123,13 @@ func (d *Daemon) handleClient(conn net.Conn) { dur := time.Duration(d.cfg.Git.MaxTimeout) * time.Second c.maxDeadline = time.Now().Add(dur) } - defer c.Close() + d.count++ d.conns[c] = struct{}{} - defer delete(d.conns, c) - - // Close connection if there are too many open connections. - if len(d.conns) >= d.cfg.Git.MaxConnections { - log.Printf("git: max connections reached, closing %s", c.RemoteAddr()) - fatal(c, git.ErrMaxConns) - c.closeCanceler() - return - } + defer c.Close() + defer func() { + d.count-- + delete(d.conns, c) + }() readc := make(chan struct{}, 1) s := pktline.NewScanner(c) @@ -194,16 +201,18 @@ func (d *Daemon) handleClient(conn net.Conn) { // Close closes the underlying listener. func (d *Daemon) Close() error { d.once.Do(func() { close(d.finished) }) + err := d.listener.Close() for c := range d.conns { c.Close() delete(d.conns, c) } - return d.listener.Close() + return err } // Shutdown gracefully shuts down the daemon. func (d *Daemon) Shutdown(ctx context.Context) error { d.once.Do(func() { close(d.finished) }) + err := d.listener.Close() finished := make(chan struct{}, 1) go func() { d.wg.Wait() @@ -213,6 +222,6 @@ func (d *Daemon) Shutdown(ctx context.Context) error { case <-ctx.Done(): return ctx.Err() case <-finished: - return d.listener.Close() + return err } } diff --git a/server/git/error.go b/server/git/error.go index 11dc0ea4e825e2d1f6eb91b5eb66da4ec6ad6b53..1c04fe4a7723ac226b819f41d56176a71f71cf09 100644 --- a/server/git/error.go +++ b/server/git/error.go @@ -14,8 +14,8 @@ var ErrInvalidRepo = errors.New("invalid repo") // ErrInvalidRequest represents an invalid request. var ErrInvalidRequest = errors.New("invalid request") -// ErrMaxConns represents a maximum connection limit being reached. -var ErrMaxConns = errors.New("too many connections, try again later") +// ErrMaxConnections represents a maximum connection limit being reached. +var ErrMaxConnections = errors.New("too many connections, try again later") // ErrTimeout is returned when the maximum read timeout is exceeded. var ErrTimeout = errors.New("I/O timeout reached")