mhale / smtpd

An SMTP server package written in Go, in the style of the built-in HTTP server.
The Unlicense
404 stars 92 forks source link

How to gracefully shutdown smtpd? #27

Open ggilley opened 3 years ago

ggilley commented 3 years ago

You mentioned that you modeled it after the built-in http server. It has a Shutdown method for gracefully shutting down the service. How do I do the equivalent with smtpd? Thanks!

mhale commented 3 years ago

When I wrote it initially, the Shutdown function for the built-in HTTP server didn't exist, and there is no current equivalent. If you'd like to submit a pull request, I'd be happy to take a look at it.

ggilley commented 3 years ago

29

cuu508 commented 3 years ago

Hello, I'm looking at implementing a graceful shutdown but can't quite get it to work. Here's my code that starts the SMTP listener:

func listenEmail(ctx context.Context, port string) {
    srv := &smtpd.Server{
        Addr: "0.0.0.0:" + port,
        Handler: mailHandler,
        Appname: "...",
        Hostname: "...",
    }

    go func() {
        <- ctx.Done()

        log.Println("We should stop the SMTP server now...")
        srv.Close()
        log.Println("We closed it")
    }()

    err := srv.ListenAndServe()
    if err != nil {
        log.Fatal("Unable to start smtp server: ", err)
        os.Exit(1)
    }

    log.Println("listenEmail goroutine quits")
}

Not shown here, but on Ctrl+C, ctx gets cancelled, so the inner goroutine runs and calls srv.Close(). When I run this, the "We closed it" message from the inner goroutine gets printed, but the final "listenEmail goroutine quits" does not. I assume that's because ListenAndServe still keeps blocking.

I was looking at the smtpd source code:

// Serve creates a new SMTP session after a network connection is established.
func (srv *Server) Serve(ln net.Listener) error {
    if atomic.LoadInt32(&srv.inShutdown) != 0 {
        return ErrServerClosed
    }

    defer ln.Close()
    for {

        // if we are shutting down, don't accept new connections
        select {
        case <-srv.getShutdownChan():
            return ErrServerClosed
        default:
        }

        conn, err := ln.Accept()
        if err != nil {
            if netErr, ok := err.(net.Error); ok && netErr.Temporary() {
                continue
            }
            return err
        }

        session := srv.newSession(conn)
        atomic.AddInt32(&srv.openSessions, 1)
        go session.serve()
    }
}

To me it looks like it checks the shutdown condition at the start of the for loop, and then blocks at ln.Accept(). So, I'm guessing, it needs to receive one extra email for the loop to advance and the Serve call to return. Does that make sense? I'm a Go novice and am stepping on rakes and making wrong guesses left and right :-)

I also run a HTTP listener (http.Server) in a similar way, and it is able to finish up immediately, without waiting for one extra HTTP request. Perhaps there's a viable way to make smtpd work the same?

mhale commented 3 years ago

Your analysis makes sense. The Server in the http package has handling for the shutdown after the Accept call which is missing here. Want to have a go at fixing it and sending a pull request?

cuu508 commented 3 years ago

I can give it a try, but it would need a thorough review for sure. My knowledge of Go is minimal.