Skip to content

Commit

Permalink
fix: Wait for handle to finish before closing session
Browse files Browse the repository at this point in the history
  • Loading branch information
jameshoulahan committed Oct 3, 2022
1 parent 838043f commit 151fe7c
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 16 deletions.
9 changes: 4 additions & 5 deletions internal/session/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,21 @@ func (s *Session) startCommandReader(ctx context.Context, del string) <-chan com
}

if err == nil && cmd.GetStartTLS() != nil {
// TLS needs to be handled here in order to ensure that next command read is over the
// tls connection.
if e := s.handleStartTLS(tag, cmd.GetStartTLS()); e != nil {
// TLS needs to be handled here to ensure that next command read is over the TLS connection.
if startTLSErr := s.handleStartTLS(tag, cmd.GetStartTLS()); startTLSErr != nil {
cmd = nil
err = e
err = startTLSErr
} else {
continue
}
}

select {
case cmdCh <- command{tag: tag, cmd: cmd, err: err}:
// ...

case <-ctx.Done():
return

}
}
})
Expand Down
6 changes: 3 additions & 3 deletions internal/session/handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ func (s *Session) handleOther(
tag string,
cmd *proto.Command,
profiler profiling.CmdProfiler,
) chan response.Response {
) <-chan response.Response {
ch := make(chan response.Response, channelBufferCount)

go func() {
s.handleWG.Go(func() {
labels := pprof.Labels("go", "handleOther()", "SessionID", strconv.Itoa(s.sessionID))
pprof.Do(ctx, labels, func(_ context.Context) {
defer close(ch)
Expand All @@ -35,7 +35,7 @@ func (s *Session) handleOther(
}
}
})
}()
})

return ch
}
Expand Down
18 changes: 16 additions & 2 deletions internal/session/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/ProtonMail/gluon/internal/state"
"github.com/ProtonMail/gluon/profiling"
"github.com/ProtonMail/gluon/version"
"github.com/ProtonMail/gluon/wait"
"github.com/sirupsen/logrus"
"golang.org/x/exp/slices"
)
Expand Down Expand Up @@ -64,20 +65,32 @@ type Session struct {
// before the client logs in or selects a mailbox.
imapID imap.IMAPID

// version is the version info of the Gluon server.
version version.Info

// cmdProfilerBuilder is used in profiling command execution.
cmdProfilerBuilder profiling.CmdProfilerBuilder

// handleWG is used to wait for all commands to finish before closing the session.
handleWG wait.Group
}

func New(conn net.Conn, backend *backend.Backend, sessionID int, versionInfo version.Info, profiler profiling.CmdProfilerBuilder, eventCh chan<- events.Event) *Session {
func New(
conn net.Conn,
backend *backend.Backend,
sessionID int,
version version.Info,
profiler profiling.CmdProfilerBuilder,
eventCh chan<- events.Event,
) *Session {
return &Session{
conn: conn,
liner: liner.New(conn),
backend: backend,
caps: []imap.Capability{imap.IMAP4rev1, imap.IDLE, imap.UNSELECT, imap.UIDPLUS, imap.MOVE},
sessionID: sessionID,
eventCh: eventCh,
version: versionInfo,
version: version,
cmdProfilerBuilder: profiler,
}
}
Expand Down Expand Up @@ -110,6 +123,7 @@ func (s *Session) SetTLSConfig(cfg *tls.Config) {

func (s *Session) Serve(ctx context.Context) error {
defer s.done(ctx)
defer s.handleWG.Wait()

if err := s.greet(); err != nil {
return err
Expand Down
5 changes: 3 additions & 2 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/ProtonMail/gluon/reporter"
"github.com/ProtonMail/gluon/store"
"github.com/ProtonMail/gluon/version"
"github.com/ProtonMail/gluon/wait"
"github.com/ProtonMail/gluon/watcher"
_ "github.com/mattn/go-sqlite3"
"github.com/sirupsen/logrus"
Expand All @@ -45,7 +46,7 @@ type Server struct {
serveDoneCh chan struct{}

// serveWG keeps track of serving goroutines.
serveWG WaitGroup
serveWG wait.Group

// nextID holds the ID that will be given to the next session.
nextID int
Expand Down Expand Up @@ -166,7 +167,7 @@ func (s *Server) Serve(ctx context.Context, l net.Listener) error {

// serve handles incoming connections and starts a new goroutine for each.
func (s *Server) serve(ctx context.Context, connCh <-chan net.Conn) {
var connWG WaitGroup
var connWG wait.Group
defer connWG.Wait()

for {
Expand Down
8 changes: 4 additions & 4 deletions wg.go → wait/wg.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
package gluon
package wait

import "sync"

type WaitGroup struct {
type Group struct {
wg sync.WaitGroup
}

func (wg *WaitGroup) Go(f func()) {
func (wg *Group) Go(f func()) {
wg.wg.Add(1)

go func() {
Expand All @@ -15,6 +15,6 @@ func (wg *WaitGroup) Go(f func()) {
}()
}

func (wg *WaitGroup) Wait() {
func (wg *Group) Wait() {
wg.wg.Wait()
}

0 comments on commit 151fe7c

Please sign in to comment.