Skip to content

Commit

Permalink
providers/darwin/process: fix error handling (#150)
Browse files Browse the repository at this point in the history
* providers/darwin/process: fix error handling

kern_procargs was not returning an error when kern.procargs2 failed.
Then meant that you didn't get an error when the process no longer exists
or when you didn't have permissions to access the process.

For process.Info(), this especially affected builds running without CGO enabled.
When running with CGO there are other calls that will fail with "no such process"
or "operation not permitted" before we get to calling kern.procargs2, but without
CGO it goes directly to calling kern.procargs2. This resulted in Info() returning
a zero value types.ProcessInfo and no error.

Fixes #148

* system_test.go: Ignore EINVAL for process.Info()

Non-CGO build will return EINVAL when a process does not exist.
  • Loading branch information
andrewkroh authored Jan 26, 2023
1 parent 2841c12 commit dbd6a9d
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 31 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- On darwin without CGO `process.Info()` could fail, but would not return the error. [#150](https://github.com/elastic/go-sysinfo/pull/150)

## [1.9.0]

### Added
Expand Down
9 changes: 8 additions & 1 deletion providers/darwin/process_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ package darwin
import (
"bytes"
"encoding/binary"
"errors"
"fmt"
"os"
"strconv"
"syscall"
"time"

"golang.org/x/sys/unix"
Expand Down Expand Up @@ -178,7 +180,12 @@ var nullTerminator = []byte{0}
func kern_procargs(pid int, p *process) error {
data, err := unix.SysctlRaw("kern.procargs2", pid)
if err != nil {
return nil
if errors.Is(err, syscall.EINVAL) {
// sysctl returns "invalid argument" for both "no such process"
// and "operation not permitted" errors.
return fmt.Errorf("no such process or operation not permitted: %w", err)
}
return err
}
buf := bytes.NewBuffer(data)

Expand Down
59 changes: 36 additions & 23 deletions providers/darwin/process_darwin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@
package darwin

import (
"errors"
"os"
"os/exec"
"syscall"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -97,33 +99,44 @@ func TestProcessEnvironmentInternal(t *testing.T) {

func TestProcesses(t *testing.T) {
var s darwinSystem
ps, err := s.Processes()
processes, err := s.Processes()
if err != nil {
t.Fatal(err)
}

pinfo, err := ps[0].Info()
if err != nil {
t.Fatal(err)
}

if pinfo.PID == 0 {
t.Fatal("empty pid")
var count int
for _, proc := range processes {
processInfo, err := proc.Info()
switch {
// Ignore processes that no longer exist or that cannot be accessed.
case errors.Is(err, syscall.ESRCH),
errors.Is(err, syscall.EPERM),
errors.Is(err, syscall.EINVAL):
continue
case err != nil:
t.Fatalf("failed to get process info for PID=%d: %v", proc.PID(), err)
default:
count++
}

if processInfo.PID == 0 {
t.Fatalf("empty pid in %#v", processInfo)
}

if processInfo.Exe == "" {
t.Fatalf("empty exec in %#v", processInfo)
}

u, err := proc.User()
require.NoError(t, err)

require.NotEmpty(t, u.UID)
require.NotEmpty(t, u.EUID)
require.NotEmpty(t, u.SUID)
require.NotEmpty(t, u.GID)
require.NotEmpty(t, u.EGID)
require.NotEmpty(t, u.SGID)
}

if pinfo.Exe == "" {
t.Fatal("empty exec")
}

u, err := ps[0].User()
require.NoError(t, err)

require.NotEmpty(t, u.UID)
require.NotEmpty(t, u.EUID)
require.NotEmpty(t, u.SUID)
require.NotEmpty(t, u.GID)
require.NotEmpty(t, u.EGID)
require.NotEmpty(t, u.SGID)

t.Log(ps[0].Info())
assert.NotZero(t, count, "failed to get process info for any processes")
}
17 changes: 10 additions & 7 deletions system_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,14 +292,17 @@ func TestProcesses(t *testing.T) {
t.Log("Found", len(procs), "processes.")
for _, proc := range procs {
info, err := proc.Info()
if err != nil {
if errors.Is(err, fs.ErrPermission) || errors.Is(err, syscall.ESRCH) {
// The process may no longer exist by the time we try fetching
// additional information so ignore ESRCH (no such process).
continue
}
t.Fatal(err)
switch {
// Ignore processes that no longer exist or that cannot be accessed.
case errors.Is(err, syscall.ESRCH),
errors.Is(err, syscall.EPERM),
errors.Is(err, syscall.EINVAL),
errors.Is(err, fs.ErrPermission):
continue
case err != nil:
t.Fatalf("failed to get process info for PID=%d: %v", proc.PID(), err)
}

t.Logf("pid=%v name='%s' exe='%s' args=%+v ppid=%d cwd='%s' start_time=%v",
info.PID, info.Name, info.Exe, info.Args, info.PPID, info.CWD,
info.StartTime)
Expand Down

0 comments on commit dbd6a9d

Please sign in to comment.