From 8bdb235b552d760613f9e97679b38029fa2e6b45 Mon Sep 17 00:00:00 2001 From: rzpt <51346452+rzpt@users.noreply.github.com> Date: Tue, 9 Jun 2020 11:12:47 -0700 Subject: [PATCH] fix process running check (#130) Fix false positive process detection when an unrelated thread's id matches the old process id. --- Gopkg.lock | 8 +- changelog/@unreleased/pr-130.v2.yml | 5 + init/cli/lib.go | 40 ++- init/cli/stop.go | 12 +- vendor/github.com/mitchellh/go-ps/.gitignore | 1 + vendor/github.com/mitchellh/go-ps/.travis.yml | 4 + vendor/github.com/mitchellh/go-ps/LICENSE.md | 21 ++ vendor/github.com/mitchellh/go-ps/README.md | 34 +++ vendor/github.com/mitchellh/go-ps/Vagrantfile | 43 +++ vendor/github.com/mitchellh/go-ps/go.mod | 3 + vendor/github.com/mitchellh/go-ps/process.go | 40 +++ .../mitchellh/go-ps/process_darwin.go | 138 ++++++++++ .../mitchellh/go-ps/process_freebsd.go | 260 ++++++++++++++++++ .../mitchellh/go-ps/process_linux.go | 35 +++ .../mitchellh/go-ps/process_solaris.go | 96 +++++++ .../mitchellh/go-ps/process_unix.go | 95 +++++++ .../mitchellh/go-ps/process_windows.go | 119 ++++++++ 17 files changed, 944 insertions(+), 10 deletions(-) create mode 100644 changelog/@unreleased/pr-130.v2.yml create mode 100644 vendor/github.com/mitchellh/go-ps/.gitignore create mode 100644 vendor/github.com/mitchellh/go-ps/.travis.yml create mode 100644 vendor/github.com/mitchellh/go-ps/LICENSE.md create mode 100644 vendor/github.com/mitchellh/go-ps/README.md create mode 100644 vendor/github.com/mitchellh/go-ps/Vagrantfile create mode 100644 vendor/github.com/mitchellh/go-ps/go.mod create mode 100644 vendor/github.com/mitchellh/go-ps/process.go create mode 100644 vendor/github.com/mitchellh/go-ps/process_darwin.go create mode 100644 vendor/github.com/mitchellh/go-ps/process_freebsd.go create mode 100644 vendor/github.com/mitchellh/go-ps/process_linux.go create mode 100644 vendor/github.com/mitchellh/go-ps/process_solaris.go create mode 100644 vendor/github.com/mitchellh/go-ps/process_unix.go create mode 100644 vendor/github.com/mitchellh/go-ps/process_windows.go diff --git a/Gopkg.lock b/Gopkg.lock index b26c526f..29dab287 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -7,6 +7,12 @@ revision = "346938d642f2ec3594ed81d874461961cd0faa76" version = "v1.1.0" +[[projects]] + name = "github.com/mitchellh/go-ps" + packages = ["."] + revision = "147ff83818ae939913b2e20b91ae3cd6c391771c" + version = "v1.0.0" + [[projects]] branch = "master" name = "github.com/mitchellh/go-wordwrap" @@ -70,6 +76,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "f5acf16fae9932555264d1e8a827ed3a8484d71ccb4e16b1dc86df885e5ac063" + inputs-digest = "04cb028c8bb597646fb5c39d30a5d686a6a513923f502866aa65d68f138946f6" solver-name = "gps-cdcl" solver-version = 1 diff --git a/changelog/@unreleased/pr-130.v2.yml b/changelog/@unreleased/pr-130.v2.yml new file mode 100644 index 00000000..cd6a468f --- /dev/null +++ b/changelog/@unreleased/pr-130.v2.yml @@ -0,0 +1,5 @@ +type: fix +fix: + description: Fix false positive process detection when an unrelated thread's id matches the old process id. + links: + - https://github.com/palantir/go-java-launcher/pull/130 diff --git a/init/cli/lib.go b/init/cli/lib.go index 18af93ae..2df5a679 100644 --- a/init/cli/lib.go +++ b/init/cli/lib.go @@ -24,6 +24,7 @@ import ( "strings" "syscall" + ps "github.com/mitchellh/go-ps" "github.com/palantir/pkg/cli" "github.com/pkg/errors" @@ -108,7 +109,11 @@ func getCmdProcess(name string) (*int, *os.Process, error) { return nil, nil, errors.Wrap(err, "pid file did not contain an integer") } - if running, proc := isPidRunning(pid); running { + running, proc, err := isPidRunning(pid) + if err != nil { + return nil, nil, err + } + if running { return &pid, proc, nil } return &pid, nil, nil @@ -146,16 +151,37 @@ func getConfiguredCommands(ctx cli.Context, loggers launchlib.ServiceLoggers) (m return cmds, nil } -func isPidRunning(pid int) (bool, *os.Process) { +func isPidRunning(pid int) (bool, *os.Process, error) { // Docs say FindProcess always succeeds on Unix. proc, _ := os.FindProcess(pid) - if isProcRunning(proc) { - return true, proc + running, err := isProcRunning(proc) + if err != nil { + return false, nil, err } - return false, nil + if running { + return true, proc, nil + } + return false, nil, nil } -func isProcRunning(proc *os.Process) bool { +func isProcRunning(proc *os.Process) (bool, error) { // This is the way to check if a process exists: https://linux.die.net/man/2/kill. - return proc.Signal(syscall.Signal(0)) == nil + // On linux, this may respond true if there is a thread running with the same id. + running := proc.Signal(syscall.Signal(0)) == nil + if !running { + return false, nil + } + + // On linux, iterating over processes will only return running processes. Unfortunately, + // getting exactly the one pid will also return a thread of the same id. + procs, err := ps.Processes() + if err != nil { + return false, err + } + for _, p := range procs { + if p.Pid() == proc.Pid { + return true, nil + } + } + return false, nil } diff --git a/init/cli/stop.go b/init/cli/stop.go index a9d381ca..4d46b9f9 100644 --- a/init/cli/stop.go +++ b/init/cli/stop.go @@ -106,7 +106,11 @@ func waitForServiceToStop(ctx cli.Context, procs map[string]*os.Process) error { select { case <-ticker.Chan(): for name, remainingProc := range procs { - if !isProcRunning(remainingProc) { + running, err := isProcRunning(remainingProc) + if err != nil { + return err + } + if !running { delete(procs, name) } } @@ -116,7 +120,11 @@ func waitForServiceToStop(ctx cli.Context, procs map[string]*os.Process) error { case <-timer.Chan(): killedProcs := make([]string, 0, len(procs)) for name, remainingProc := range procs { - if isProcRunning(remainingProc) { + running, err := isProcRunning(remainingProc) + if err != nil { + return err + } + if running { if err := remainingProc.Kill(); err != nil { // If this actually errors, something is probably seriously wrong. // Just stop immediately. diff --git a/vendor/github.com/mitchellh/go-ps/.gitignore b/vendor/github.com/mitchellh/go-ps/.gitignore new file mode 100644 index 00000000..a977916f --- /dev/null +++ b/vendor/github.com/mitchellh/go-ps/.gitignore @@ -0,0 +1 @@ +.vagrant/ diff --git a/vendor/github.com/mitchellh/go-ps/.travis.yml b/vendor/github.com/mitchellh/go-ps/.travis.yml new file mode 100644 index 00000000..8f794f71 --- /dev/null +++ b/vendor/github.com/mitchellh/go-ps/.travis.yml @@ -0,0 +1,4 @@ +language: go + +go: + - 1.2.1 diff --git a/vendor/github.com/mitchellh/go-ps/LICENSE.md b/vendor/github.com/mitchellh/go-ps/LICENSE.md new file mode 100644 index 00000000..22985159 --- /dev/null +++ b/vendor/github.com/mitchellh/go-ps/LICENSE.md @@ -0,0 +1,21 @@ +The MIT License (MIT) + +Copyright (c) 2014 Mitchell Hashimoto + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. diff --git a/vendor/github.com/mitchellh/go-ps/README.md b/vendor/github.com/mitchellh/go-ps/README.md new file mode 100644 index 00000000..4e3d0e14 --- /dev/null +++ b/vendor/github.com/mitchellh/go-ps/README.md @@ -0,0 +1,34 @@ +# Process List Library for Go [![GoDoc](https://godoc.org/github.com/mitchellh/go-ps?status.png)](https://godoc.org/github.com/mitchellh/go-ps) + +go-ps is a library for Go that implements OS-specific APIs to list and +manipulate processes in a platform-safe way. The library can find and +list processes on Linux, Mac OS X, Solaris, and Windows. + +If you're new to Go, this library has a good amount of advanced Go educational +value as well. It uses some advanced features of Go: build tags, accessing +DLL methods for Windows, cgo for Darwin, etc. + +How it works: + + * **Darwin** uses the `sysctl` syscall to retrieve the process table. + * **Unix** uses the procfs at `/proc` to inspect the process tree. + * **Windows** uses the Windows API, and methods such as + `CreateToolhelp32Snapshot` to get a point-in-time snapshot of + the process table. + +## Installation + +Install using standard `go get`: + +``` +$ go get github.com/mitchellh/go-ps +... +``` + +## TODO + +Want to contribute? Here is a short TODO list of things that aren't +implemented for this library that would be nice: + + * FreeBSD support + * Plan9 support diff --git a/vendor/github.com/mitchellh/go-ps/Vagrantfile b/vendor/github.com/mitchellh/go-ps/Vagrantfile new file mode 100644 index 00000000..61662ab1 --- /dev/null +++ b/vendor/github.com/mitchellh/go-ps/Vagrantfile @@ -0,0 +1,43 @@ +# -*- mode: ruby -*- +# vi: set ft=ruby : + +# Vagrantfile API/syntax version. Don't touch unless you know what you're doing! +VAGRANTFILE_API_VERSION = "2" + +Vagrant.configure(VAGRANTFILE_API_VERSION) do |config| + config.vm.box = "chef/ubuntu-12.04" + + config.vm.provision "shell", inline: $script + + ["vmware_fusion", "vmware_workstation"].each do |p| + config.vm.provider "p" do |v| + v.vmx["memsize"] = "1024" + v.vmx["numvcpus"] = "2" + v.vmx["cpuid.coresPerSocket"] = "1" + end + end +end + +$script = <