Skip to content

Commit

Permalink
log.LogPanicAndExit is callers responsibility
Browse files Browse the repository at this point in the history
By using the same pattern everywhere we can easily check for go routines that do not call log.LogPanicAndExit.

Also this commit adds a few missing calls in fetcher and deduper and possibly more.
  • Loading branch information
lukedirtwalker committed Oct 29, 2018
1 parent abdf30d commit bb7f692
Show file tree
Hide file tree
Showing 34 changed files with 176 additions and 63 deletions.
2 changes: 1 addition & 1 deletion go/border/error.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Copyright 2016 ETH Zurich
// Copyright 2018 ETH Zurich, Anapaya Systems
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -48,7 +49,6 @@ func (r *Router) handlePktError(rp *rpkt.RtrPkt, perr error, desc string) {

// PackeError creates an SCMP error for the given packet and sends it to its source.
func (r *Router) PacketError() {
defer log.LogPanicAndExit()
// Run forever.
for args := range r.pktErrorQ {
r.doPktError(args.rp, args.perr)
Expand Down
6 changes: 5 additions & 1 deletion go/border/io-hsr.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Copyright 2016 ETH Zurich
// Copyright 2018 ETH Zurich, Anapaya Systems
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -43,7 +44,10 @@ type HSRInput struct {

func (hi *HSRInput) Start() {
if !hi.running {
go hi.Func(hi.Router, hi.StopChan, hi.StoppedChan)
go func() {
defer log.LogPanicAndExit()
hi.Func(hi.Router, hi.StopChan, hi.StoppedChan)
}()
hi.running = true
}
}
Expand Down
2 changes: 2 additions & 0 deletions go/border/main.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Copyright 2016 ETH Zurich
// Copyright 2018 ETH Zurich, Anapaya Systems
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -89,6 +90,7 @@ func setupSignals() {
signal.Notify(sig, os.Interrupt)
signal.Notify(sig, syscall.SIGTERM)
go func() {
defer log.LogPanicAndExit()
<-sig
log.Info("Exiting")
profile.Stop()
Expand Down
6 changes: 5 additions & 1 deletion go/border/metrics/metrics.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Copyright 2016 ETH Zurich
// Copyright 2018 ETH Zurich, Anapaya Systems
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -144,6 +145,9 @@ func Start() error {
return common.NewBasicError("Unable to bind prometheus metrics port", err)
}
log.Info("Exporting prometheus metrics", "addr", *promAddr)
go http.Serve(ln, nil)
go func() {
defer log.LogPanicAndExit()
http.Serve(ln, nil)
}()
return nil
}
13 changes: 9 additions & 4 deletions go/border/rctrl/ctrl.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2018 ETH Zurich
// Copyright 2018 ETH Zurich, Anapaya Systems
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -41,7 +41,6 @@ var (
)

func Control(sRevInfoQ chan rpkt.RawSRevCallbackArgs) {
defer log.LogPanicAndExit()
var err error
logger = log.New("Part", "Control")
ctx := rctx.Get()
Expand All @@ -61,8 +60,14 @@ func Control(sRevInfoQ chan rpkt.RawSRevCallbackArgs) {
logger.Error("Listening on address", "addr", ctrlAddr, "err", err)
return
}
go ifStateUpdate()
go revInfoFwd(sRevInfoQ)
go func() {
defer log.LogPanicAndExit()
ifStateUpdate()
}()
go func() {
defer log.LogPanicAndExit()
revInfoFwd(sRevInfoQ)
}()
processCtrl()
}

Expand Down
4 changes: 1 addition & 3 deletions go/border/rctrl/ifstate.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2018 ETH Zurich
// Copyright 2018 ETH Zurich, Anapaya Systems
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand All @@ -21,7 +21,6 @@ import (
"github.com/scionproto/scion/go/lib/addr"
"github.com/scionproto/scion/go/lib/ctrl"
"github.com/scionproto/scion/go/lib/ctrl/path_mgmt"
"github.com/scionproto/scion/go/lib/log"
"github.com/scionproto/scion/go/lib/snet"
)

Expand All @@ -39,7 +38,6 @@ const (
// interface state changes, so this is only needed as a fail-safe after
// startup.
func ifStateUpdate() {
defer log.LogPanicAndExit()
genIFStateReq()
for range time.Tick(ifStateFreq) {
genIFStateReq()
Expand Down
3 changes: 1 addition & 2 deletions go/border/rctrl/revinfo.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2018 ETH Zurich
// Copyright 2018 ETH Zurich, Anapaya Systems
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -27,7 +27,6 @@ import (
// RevInfoFwd takes RevInfos, and forwards them to the local Beacon Service
// (BS) and Path Service (PS).
func revInfoFwd(revInfoQ chan rpkt.RawSRevCallbackArgs) {
defer log.LogPanicAndExit()
// Run forever.
for args := range revInfoQ {
revInfo, err := args.SignedRevInfo.RevInfo()
Expand Down
11 changes: 9 additions & 2 deletions go/border/rctx/io.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Copyright 2017 ETH Zurich
// Copyright 2018 ETH Zurich, Anapaya Systems
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -74,10 +75,16 @@ func NewSock(ring *ringbuf.Ring, conn conn.Conn, dir rcmn.Dir,
func (s *Sock) Start() {
if !s.running {
if s.Reader != nil {
go s.Reader(s, s.stop, s.readerStopped)
go func() {
defer log.LogPanicAndExit()
s.Reader(s, s.stop, s.readerStopped)
}()
}
if s.Writer != nil {
go s.Writer(s, s.stop, s.writerStopped)
go func() {
defer log.LogPanicAndExit()
s.Writer(s, s.stop, s.writerStopped)
}()
}
s.running = true
log.Info("Sock routines started", "addr", s.Conn.LocalAddr())
Expand Down
16 changes: 12 additions & 4 deletions go/border/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,18 @@ func NewRouter(id, confDir string) (*Router, error) {
// Run sets up networking, and starts go routines for handling the main packet
// processing as well as various other router functions.
func (r *Router) Run() error {
go r.PacketError()
go r.confSig()
go rctrl.Control(r.sRevInfoQ)
go func() {
defer log.LogPanicAndExit()
r.PacketError()
}()
go func() {
defer log.LogPanicAndExit()
r.confSig()
}()
go func() {
defer log.LogPanicAndExit()
rctrl.Control(r.sRevInfoQ)
}()
// TODO(shitz): Here should be some code to periodically check the discovery
// service for updated info.
var wait chan struct{}
Expand All @@ -83,7 +92,6 @@ func (r *Router) Run() error {

// confSig handles reloading the configuration when SIGHUP is received.
func (r *Router) confSig() {
defer log.LogPanicAndExit()
for range sighup {
var err error
var config *conf.Conf
Expand Down
7 changes: 5 additions & 2 deletions go/cert_srv/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,16 +105,19 @@ func setupSignals() {
signal.Notify(sig, os.Interrupt)
signal.Notify(sig, syscall.SIGTERM)
go func() {
defer log.LogPanicAndExit()
s := <-sig
log.Info("Received signal, exiting...", "signal", s)
log.Flush()
os.Exit(1)
}()
go configSig()
go func() {
defer log.LogPanicAndExit()
configSig()
}()
}

func configSig() {
defer log.LogPanicAndExit()
for range sighup {
log.Info("Reloading is not supported")
}
Expand Down
5 changes: 4 additions & 1 deletion go/cert_srv/reiss_tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,10 @@ func (r *ReissRequester) Run() {
}

ctx, cancelF := context.WithTimeout(context.Background(), DefaultReissTimeout)
go r.sendReq(ctx, cancelF, chain, config)
go func() {
defer log.LogPanicAndExit()
r.sendReq(ctx, cancelF, chain, config)
}()
time.Sleep(config.ReissRate)
}
}
Expand Down
11 changes: 9 additions & 2 deletions go/examples/pingpong/pingpong.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Copyright 2017 ETH Zurich
// Copyright 2018 ETH Zurich, Anapaya Systems
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -246,7 +247,10 @@ func (c *client) run() {
}
c.quicStream = newQuicStream(qstream)
log.Debug("Quic stream opened", "local", &local, "remote", &remote)
go c.send()
go func() {
defer log.LogPanicAndExit()
c.send()
}()
c.read()
}

Expand Down Expand Up @@ -359,7 +363,10 @@ func (s server) run() {
break
}
log.Info("Quic session accepted", "src", qsess.RemoteAddr())
go s.handleClient(qsess)
go func() {
defer log.LogPanicAndExit()
s.handleClient(qsess)
}()
}
}

Expand Down
8 changes: 7 additions & 1 deletion go/lib/infra/dedupe/dedupe.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ package dedupe
import (
"context"
"time"

"github.com/scionproto/scion/go/lib/log"
)

const (
Expand Down Expand Up @@ -135,7 +137,10 @@ func New(f RequestFunc, dedupeLifetime, responseValidity time.Duration) Deduper
func (dd *deduper) Request(ctx context.Context, req Request) (<-chan Response, CancelFunc) {
ch := make(chan Response, 1)
if ctx := dd.notifications.Add(req, ch, dd.dedupeLifetime); ctx != nil {
go dd.handler(ctx, req)
go func() {
defer log.LogPanicAndExit()
dd.handler(ctx, req)
}()
}

return ch, func() {
Expand All @@ -150,6 +155,7 @@ func (dd *deduper) Request(ctx context.Context, req Request) (<-chan Response, C
func (dd *deduper) handler(ctx context.Context, req Request) {
ch := make(chan Response, 1)
go func() {
defer log.LogPanicAndExit()
ch <- dd.requestFunc(ctx, req)
}()

Expand Down
7 changes: 5 additions & 2 deletions go/lib/infra/example/main.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2018 ETH Zurich
// Copyright 2018 ETH Zurich, Anapaya Systems
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -46,7 +46,10 @@ func main() {
serverApp.trustStore.NewChainReqHandler(false))
serverApp.messenger.AddHandler(infra.TRCRequest,
serverApp.trustStore.NewTRCReqHandler(false))
go serverApp.messenger.ListenAndServe()
go func() {
defer log.LogPanicAndExit()
serverApp.messenger.ListenAndServe()
}()
// Do work
select {}
}
Expand Down
2 changes: 1 addition & 1 deletion go/lib/infra/messenger/messenger.go
Original file line number Diff line number Diff line change
Expand Up @@ -557,8 +557,8 @@ func (m *Messenger) serve(ctx context.Context, cancelF context.CancelFunc, pld *
return
}
go func() {
defer cancelF()
defer log.LogPanicAndExit()
defer cancelF()
handler.Handle(infra.NewRequest(ctx, msg, signedPld, address, pld.ReqId, logger))
}()
}
Expand Down
10 changes: 8 additions & 2 deletions go/lib/infra/modules/trust/trust_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,10 @@ func TestTRCReqHandler(t *testing.T) {
Convey(tc.Name, func() {
handler := store.NewTRCReqHandler(tc.RecursionEnabled)
serverMessenger.AddHandler(infra.TRCRequest, handler)
go serverMessenger.ListenAndServe()
go func() {
defer log.LogPanicAndExit()
serverMessenger.ListenAndServe()
}()
defer serverMessenger.CloseServer()

ctx, cancelF := context.WithTimeout(context.Background(), testCtxTimeout)
Expand Down Expand Up @@ -685,7 +688,10 @@ func TestChainReqHandler(t *testing.T) {
Convey(tc.Name, func() {
handler := store.NewChainReqHandler(tc.RecursionEnabled)
serverMessenger.AddHandler(infra.ChainRequest, handler)
go serverMessenger.ListenAndServe()
go func() {
defer log.LogPanicAndExit()
serverMessenger.ListenAndServe()
}()
defer serverMessenger.CloseServer()

ctx, cancelF := context.WithTimeout(context.Background(), testCtxTimeout)
Expand Down
12 changes: 8 additions & 4 deletions go/lib/integration/binary.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,10 @@ func (bi *binaryIntegration) StartServer(ctx context.Context, dst addr.IA) (Wait
}
}
}()
go bi.logRedirect("Server", "ServerErr", dst, ep)
go func() {
defer log.LogPanicAndExit()
bi.logRedirect("Server", "ServerErr", dst, ep)
}()
err = r.Start()
if err != nil {
return nil, err
Expand All @@ -127,7 +130,10 @@ func (bi *binaryIntegration) StartClient(ctx context.Context, src, dst addr.IA)
if err != nil {
return nil, err
}
go bi.logRedirect("Client", "ClientErr", src, ep)
go func() {
defer log.LogPanicAndExit()
bi.logRedirect("Client", "ClientErr", src, ep)
}()
return r, r.Start()
}

Expand All @@ -147,7 +153,6 @@ type LogRedirect func(name, pName string, local addr.IA, ep io.ReadCloser)
// StdLog tries to parse any log line from the standard format and logs it with the same log level
// as the original log entry to the log file.
var StdLog LogRedirect = func(name, pName string, local addr.IA, ep io.ReadCloser) {
defer log.LogPanicAndExit()
defer ep.Close()
logparse.ParseFrom(ep, pName, pName, func(e logparse.LogEntry) {
log.Log(e.Level, fmt.Sprintf("%s@%s: %s", name, local, strings.Join(e.Lines, "\n")))
Expand All @@ -156,7 +161,6 @@ var StdLog LogRedirect = func(name, pName string, local addr.IA, ep io.ReadClose

// NonStdLog directly logs any lines as error to the log file
var NonStdLog LogRedirect = func(name, pName string, local addr.IA, ep io.ReadCloser) {
defer log.LogPanicAndExit()
defer ep.Close()
scanner := bufio.NewScanner(ep)
for scanner.Scan() {
Expand Down
1 change: 1 addition & 0 deletions go/lib/log/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ func SetupLogFile(name string, logDir string, logLevel string, logSize int, logA

if logFlush > 0 {
go func() {
defer LogPanicAndExit()
for range time.Tick(time.Duration(logFlush) * time.Second) {
Flush()
}
Expand Down
6 changes: 5 additions & 1 deletion go/lib/pathmgr/pathmgr.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Copyright 2017 ETH Zurich
// Copyright 2018 ETH Zurich, Anapaya Systems
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -129,7 +130,10 @@ func New(srvc sciond.Service, timers *Timers, logger log.Logger) (*PR, error) {
normalRefire: timers.NormalRefire,
errorRefire: timers.ErrorRefire,
}
go r.run()
go func() {
defer log.LogPanicAndExit()
r.run()
}()
return pr, nil
}

Expand Down
Loading

0 comments on commit bb7f692

Please sign in to comment.