From 23ec8bcd4ff841d9d2be6a50d96939e38a574c11 Mon Sep 17 00:00:00 2001 From: Andrew Meredith Date: Fri, 30 Apr 2021 12:57:10 -0600 Subject: [PATCH 1/2] Try to stop subprocess gracefully --- internal/build.go | 2 +- internal/run.go | 26 ++++++++++++++++++++------ internal/watch.go | 20 +++++++++++++++----- 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/internal/build.go b/internal/build.go index da0f17f..8bcbf30 100644 --- a/internal/build.go +++ b/internal/build.go @@ -172,7 +172,7 @@ func (proc *funcProcess) Start() error { return proc.start() } -func (proc *funcProcess) Kill() error { +func (proc *funcProcess) Stop() error { return nil } diff --git a/internal/run.go b/internal/run.go index dbd6a63..fab4e61 100644 --- a/internal/run.go +++ b/internal/run.go @@ -23,10 +23,15 @@ import ( "os" "os/exec" "path" + "runtime" + "syscall" + "time" "github.com/evanw/esbuild/pkg/api" ) +var maxProcStopWait = 5 * time.Second + type RunOptions struct { Watch bool Entrypoint string @@ -34,10 +39,7 @@ type RunOptions struct { BuildOnly bool } -// TODO: Need to handle interrupts in order to have a higher chance -// of cleaning up temporary files. - -// Status code may be returend within an exec.ExitError return value. +// Status code may be returned within an exec.ExitError return value. func Run(repo *Repository, opts RunOptions) error { if err := EnsureTmp(repo); err != nil { return err @@ -118,6 +120,7 @@ if (typeof main === 'function') { node.Stdin = os.Stdin node.Stdout = os.Stdout node.Stderr = os.Stderr + node.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} return &cmdProcess{cmd: node} }, @@ -132,11 +135,22 @@ func (proc *cmdProcess) Start() error { return proc.cmd.Start() } -func (proc *cmdProcess) Kill() error { +func (proc *cmdProcess) Stop() error { if proc.cmd.Process == nil { return nil } - return proc.cmd.Process.Kill() + + if runtime.GOOS == "windows" { + return proc.cmd.Process.Kill() + } + + go func() { + // TODO: Make the wait time configurable. + time.Sleep(maxProcStopWait) + _ = syscall.Kill(-proc.cmd.Process.Pid, syscall.SIGKILL) + }() + + return syscall.Kill(-proc.cmd.Process.Pid, syscall.SIGTERM) } func (proc *cmdProcess) Wait() error { diff --git a/internal/watch.go b/internal/watch.go index 5abf531..0e8f91d 100644 --- a/internal/watch.go +++ b/internal/watch.go @@ -22,7 +22,7 @@ type buildAndWatch struct { type process interface { Start() error Wait() error - Kill() error + Stop() error } func (opts buildAndWatch) Run() error { @@ -86,6 +86,11 @@ func (opts buildAndWatch) Run() error { for { proc := opts.CreateProcess() done := make(chan error, 1) + waitDone := func() { + if err := <-done; err != nil { + fmt.Fprintf(os.Stderr, "could not wait for process to finish: %v\n", err) + } + } buildOK := len(result.Errors) == 0 shouldStart := buildOK && !waitForChange @@ -99,13 +104,16 @@ func (opts buildAndWatch) Run() error { } else { go func() { done <- proc.Wait() + fmt.Println("Done waiting") }() } } select { case <-abort: - if err := proc.Kill(); err != nil { - fmt.Fprintf(os.Stderr, "could not kill: %v\n", err) + if err := proc.Stop(); err != nil { + fmt.Fprintf(os.Stderr, "could not stop: %v\n", err) + } else { + waitDone() } return nil case <-restart: @@ -119,8 +127,10 @@ func (opts buildAndWatch) Run() error { break loop } } - if err := proc.Kill(); err != nil { - fmt.Fprintf(os.Stderr, "could not kill: %v\n", err) + if err := proc.Stop(); err != nil { + fmt.Fprintf(os.Stderr, "could not stop: %v\n", err) + } else { + waitDone() } result = result.Rebuild() waitForChange = false From 9626b3468def261a165e5cea00581c7d9fb062c1 Mon Sep 17 00:00:00 2001 From: Andrew Meredith Date: Fri, 30 Apr 2021 14:14:35 -0600 Subject: [PATCH 2/2] Wait for subprocess to finish when uni is stopped --- cmd/build.go | 2 +- cmd/run.go | 14 +++++++++++++- internal/build.go | 5 +++-- internal/run.go | 5 +++-- internal/watch.go | 12 ++++++++++-- 5 files changed, 30 insertions(+), 8 deletions(-) diff --git a/cmd/build.go b/cmd/build.go index 31ebd66..42a266c 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -47,7 +47,7 @@ Given no arguments, builds all packages. Otherwise, builds only the specified pa // TODO: Parallelism. for _, pkg := range packages { buildOpts.Package = pkg - if err := internal.Build(repo, buildOpts); err != nil { + if err := internal.Build(cmd.Context(), repo, buildOpts); err != nil { return err } } diff --git a/cmd/run.go b/cmd/run.go index 84c790a..08bce0f 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -1,11 +1,14 @@ package cmd import ( + "context" "errors" "fmt" "os" "os/exec" + "os/signal" "path/filepath" + "syscall" "github.com/deref/uni/internal" "github.com/spf13/cobra" @@ -61,7 +64,16 @@ export const main = async (...args: string[]) => { runOpts.Args = args[1:] - err = internal.Run(repo, runOpts) + ctx, cancel := context.WithCancel(cmd.Context()) + shutdown := make(chan os.Signal, 1) + signal.Notify(shutdown, syscall.SIGINT) + go func() { + _ = <-shutdown + fmt.Fprintf(os.Stderr, "Received shutdown signal. Waiting for process to finish.\n") + cancel() + }() + + err = internal.Run(ctx, repo, runOpts) var exitErr *exec.ExitError if errors.As(err, &exitErr) { os.Exit(exitErr.ExitCode()) diff --git a/internal/build.go b/internal/build.go index 8bcbf30..a5786d6 100644 --- a/internal/build.go +++ b/internal/build.go @@ -1,6 +1,7 @@ package internal import ( + "context" "fmt" "io/ioutil" "os" @@ -17,7 +18,7 @@ type BuildOptions struct { Watch bool } -func Build(repo *Repository, opts BuildOptions) error { +func Build(ctx context.Context, repo *Repository, opts BuildOptions) error { pkg := opts.Package packageDir := path.Join(repo.OutDir, "dist", pkg.Name) @@ -161,7 +162,7 @@ void (async () => { }, } }, - }.Run() + }.Run(ctx) } type funcProcess struct { diff --git a/internal/run.go b/internal/run.go index fab4e61..b317579 100644 --- a/internal/run.go +++ b/internal/run.go @@ -18,6 +18,7 @@ package internal import ( + "context" "fmt" "io/ioutil" "os" @@ -40,7 +41,7 @@ type RunOptions struct { } // Status code may be returned within an exec.ExitError return value. -func Run(repo *Repository, opts RunOptions) error { +func Run(ctx context.Context, repo *Repository, opts RunOptions) error { if err := EnsureTmp(repo); err != nil { return err } @@ -124,7 +125,7 @@ if (typeof main === 'function') { return &cmdProcess{cmd: node} }, - }.Run() + }.Run(ctx) } type cmdProcess struct { diff --git a/internal/watch.go b/internal/watch.go index 0e8f91d..1570ddc 100644 --- a/internal/watch.go +++ b/internal/watch.go @@ -1,6 +1,7 @@ package internal import ( + "context" "fmt" "log" "os" @@ -25,7 +26,7 @@ type process interface { Stop() error } -func (opts buildAndWatch) Run() error { +func (opts buildAndWatch) Run(ctx context.Context) error { repo := opts.Repository plugins := append([]api.Plugin{}, opts.Esbuild.Plugins...) @@ -104,7 +105,6 @@ func (opts buildAndWatch) Run() error { } else { go func() { done <- proc.Wait() - fmt.Println("Done waiting") }() } } @@ -162,9 +162,17 @@ func (opts buildAndWatch) Run() error { close(abort) return err } + case <-ctx.Done(): + close(abort) + return nil } } }) + } else { + go func() { + <-ctx.Done() + close(abort) + }() } return g.Wait()