Skip to content

Commit

Permalink
Don't cancel context on SIGPIPE in "zed serve" (#4587)
Browse files Browse the repository at this point in the history
On Linux (but not macOS), "zed serve" exits if it writes to a closed
socket (i.e., a broken network connection) because that raises SIGPIPE,
canceling the context created by cmd/zed/serve.Command.Init due to the
change in #4180.  Fix this by removing SIGPIPE from the list of
signal.NotifyContext signals for "zed serve".

Closes #4498.
  • Loading branch information
nwt authored May 10, 2023
1 parent 0d949c4 commit 6a74ad7
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 11 deletions.
11 changes: 9 additions & 2 deletions cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,15 @@ type Initializer interface {
Init() error
}

// Init is equivalent to InitWithSignals with SIGINT, SIGPIPE, and SIGTERM.
func (f *Flags) Init(all ...Initializer) (context.Context, context.CancelFunc, error) {
return f.InitWithSignals(all, syscall.SIGINT, syscall.SIGPIPE, syscall.SIGTERM)
}

// InitWithSignals handles the flags defined in SetFlags, calls the Init method
// for each element of all, and returns a context canceled when any signal in
// signals is raised.
func (f *Flags) InitWithSignals(all []Initializer, signals ...os.Signal) (context.Context, context.CancelFunc, error) {
if f.showVersion {
fmt.Printf("Version: %s\n", Version())
os.Exit(0)
Expand All @@ -47,8 +55,7 @@ func (f *Flags) Init(all ...Initializer) (context.Context, context.CancelFunc, e
if f.cpuprofile != "" {
f.runCPUProfile(f.cpuprofile)
}
ctx, cancel := signal.NotifyContext(
context.Background(), syscall.SIGINT, syscall.SIGPIPE, syscall.SIGTERM)
ctx, cancel := signal.NotifyContext(context.Background(), signals...)
cleanup := func() {
cancel()
f.cleanup()
Expand Down
11 changes: 3 additions & 8 deletions cmd/zed/root/command.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package root

import (
"context"
"flag"

"github.com/brimdata/zed/cli"
Expand All @@ -21,23 +20,19 @@ querying, and orchestrating Zed data lakes.`,

type Command struct {
charm.Command
cli.Flags
LakeFlags lakeflags.Flags
cli cli.Flags
}

func New(parent charm.Command, f *flag.FlagSet) (charm.Command, error) {
c := &Command{}
c.cli.SetFlags(f)
c.SetFlags(f)
c.LakeFlags.SetFlags(f)
return c, nil
}

func (c *Command) Init(all ...cli.Initializer) (context.Context, func(), error) {
return c.cli.Init(all...)
}

func (c *Command) Run(args []string) error {
_, cancel, err := c.cli.Init()
_, cancel, err := c.Init()
if err != nil {
return err
}
Expand Down
5 changes: 4 additions & 1 deletion cmd/zed/serve/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"os"
"os/signal"
"runtime"
"syscall"

"github.com/brimdata/zed/cli"
"github.com/brimdata/zed/cli/logflags"
Expand Down Expand Up @@ -65,7 +66,9 @@ func New(parent charm.Command, f *flag.FlagSet) (charm.Command, error) {
}

func (c *Command) Run(args []string) error {
ctx, cleanup, err := c.Init()
// Don't include SIGPIPE here or else a write to a closed socket (i.e.,
// a broken network connection) will cancel the context on Linux.
ctx, cleanup, err := c.InitWithSignals(nil, syscall.SIGINT, syscall.SIGTERM)
if err != nil {
return err
}
Expand Down

0 comments on commit 6a74ad7

Please sign in to comment.