From 43bc21468d993029d39a146d9158496721754b8a Mon Sep 17 00:00:00 2001 From: Mariana Date: Thu, 18 Apr 2019 17:48:30 +0200 Subject: [PATCH 01/10] Remove wmi queries with win32 api calls Fixes https://github.com/elastic/beats/issues/11840 Addresses high CPU load on windows --- sigar_windows.go | 63 +++++++-------- sys/windows/syscall_windows.go | 116 ++++++++++++++++++++++++++++ sys/windows/syscall_windows_test.go | 45 +++++++++++ sys/windows/zsyscall_windows.go | 29 ++++++- 4 files changed, 215 insertions(+), 38 deletions(-) diff --git a/sigar_windows.go b/sigar_windows.go index 6da30808a..3a5ba2a9b 100644 --- a/sigar_windows.go +++ b/sigar_windows.go @@ -12,7 +12,6 @@ import ( "syscall" "time" - "github.com/StackExchange/wmi" "github.com/elastic/gosigar/sys/windows" "github.com/pkg/errors" ) @@ -83,11 +82,12 @@ func (self *Uptime) Get() error { bootTimeLock.Lock() defer bootTimeLock.Unlock() if bootTime == nil { - os, err := getWin32OperatingSystem() + uptime, err := windows.GetTickCount64() if err != nil { return errors.Wrap(err, "failed to get boot time using WMI") } - bootTime = &os.LastBootUpTime + var boot = time.Unix(int64(uptime), 0) + bootTime = &boot } self.Length = time.Since(*bootTime).Seconds() @@ -251,7 +251,7 @@ func getProcStatus(pid int) (RunState, error) { var exitCode uint32 err = syscall.GetExitCodeProcess(handle, &exitCode) if err != nil { - return RunStateUnknown, errors.Wrapf(err, "GetExitCodeProcess failed for pid=%v") + return RunStateUnknown, errors.Wrapf(err, "GetExitCodeProcess failed for pid=%v", pid) } if exitCode == 259 { //still active @@ -371,15 +371,33 @@ func (self *ProcArgs) Get(pid int) error { if !version.IsWindowsVistaOrGreater() { return ErrNotImplemented{runtime.GOOS} } - - process, err := getWin32Process(int32(pid)) + handle, err := syscall.OpenProcess(processQueryLimitedInfoAccess|windows.PROCESS_VM_READ, false, uint32(pid)) + if err != nil { + return errors.Wrapf(err, "OpenProcess failed for pid=%v", pid) + } + defer syscall.CloseHandle(handle) + var args []string + pbi, err := windows.NtQueryProcessBasicInformation(handle) if err != nil { - return errors.Wrapf(err, "ProcArgs failed for pid=%v", pid) + return errors.Wrapf(err, "NtQueryProcessBasicInformation failed for pid=%v", pid) + } + if err != nil { + return nil + } + userProcParams, err := windows.GetUserProcessParams(handle, pbi) + if err != nil { + return nil + } + if argsW, err := windows.ReadProcessUnicodeString(handle, &userProcParams.CommandLine); err == nil { + args, err = windows.ByteSliceToStringSlice(argsW) + if err != nil { + args = nil + } } + var process = Win32_Process{CommandLine: &args[0]} if process.CommandLine != nil { self.List = []string{*process.CommandLine} } - return nil } @@ -396,35 +414,6 @@ func (self *FileSystemUsage) Get(path string) error { return nil } -// getWin32Process gets information about the process with the given process ID. -// It uses a WMI query to get the information from the local system. -func getWin32Process(pid int32) (Win32_Process, error) { - var dst []Win32_Process - query := fmt.Sprintf("WHERE ProcessId = %d", pid) - q := wmi.CreateQuery(&dst, query) - err := wmi.Query(q, &dst) - if err != nil { - return Win32_Process{}, fmt.Errorf("could not get Win32_Process %s: %v", query, err) - } - if len(dst) < 1 { - return Win32_Process{}, fmt.Errorf("could not get Win32_Process %s: Process not found", query) - } - return dst[0], nil -} - -func getWin32OperatingSystem() (Win32_OperatingSystem, error) { - var dst []Win32_OperatingSystem - q := wmi.CreateQuery(&dst, "") - err := wmi.Query(q, &dst) - if err != nil { - return Win32_OperatingSystem{}, errors.Wrap(err, "wmi query for Win32_OperatingSystem failed") - } - if len(dst) != 1 { - return Win32_OperatingSystem{}, errors.New("wmi query for Win32_OperatingSystem failed") - } - return dst[0], nil -} - func (self *Rusage) Get(who int) error { if who != 0 { return ErrNotImplemented{runtime.GOOS} diff --git a/sys/windows/syscall_windows.go b/sys/windows/syscall_windows.go index 36be45b30..d1222d29c 100644 --- a/sys/windows/syscall_windows.go +++ b/sys/windows/syscall_windows.go @@ -22,6 +22,11 @@ const ( PROCESS_QUERY_LIMITED_INFORMATION uint32 = 0x1000 PROCESS_VM_READ uint32 = 0x0010 ) +const ( + // SizeOfRtlUserProcessParameters gives the size + // of the RtlUserProcessParameters struct. + SizeOfRtlUserProcessParameters = unsafe.Sizeof(RtlUserProcessParameters{}) +) // MAX_PATH is the maximum length for a path in Windows. // https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx @@ -43,6 +48,26 @@ const ( DRIVE_RAMDISK ) +// UnicodeString is Go's equivalent for the _UNICODE_STRING struct. +type UnicodeString struct { + Size uint16 + MaximumLength uint16 + Buffer uintptr +} + +// RtlUserProcessParameters is Go's equivalent for the +// _RTL_USER_PROCESS_PARAMETERS struct. +// A few undocumented fields are exposed. +type RtlUserProcessParameters struct { + Reserved1 [16]byte + Reserved2 [5]uintptr + CurrentDirectoryPath UnicodeString + CurrentDirectoryHandle uintptr + DllPath UnicodeString + ImagePathName UnicodeString + CommandLine UnicodeString +} + func (dt DriveType) String() string { names := map[DriveType]string{ DRIVE_UNKNOWN: "unknown", @@ -441,6 +466,95 @@ func UTF16SliceToStringSlice(buffer []uint16) []string { return result } +func GetUserProcessParams(handle syscall.Handle, pbi ProcessBasicInformation) (params RtlUserProcessParameters, err error) { + const is32bitProc = unsafe.Sizeof(uintptr(0)) == 4 + + // Offset of params field within PEB structure. + // This structure is different in 32 and 64 bit. + paramsOffset := 0x20 + if is32bitProc { + paramsOffset = 0x10 + } + + // Read the PEB from the target process memory + pebSize := paramsOffset + 8 + peb := make([]byte, pebSize) + nRead, err := ReadProcessMemory(handle, pbi.PebBaseAddress, peb) + if err != nil { + return params, err + } + if nRead != uintptr(pebSize) { + return params, errors.Errorf("PEB: short read (%d/%d)", nRead, pebSize) + } + + // Get the RTL_USER_PROCESS_PARAMETERS struct pointer from the PEB + paramsAddr := *(*uintptr)(unsafe.Pointer(&peb[paramsOffset])) + + // Read the RTL_USER_PROCESS_PARAMETERS from the target process memory + paramsBuf := make([]byte, SizeOfRtlUserProcessParameters) + nRead, err = ReadProcessMemory(handle, paramsAddr, paramsBuf) + if err != nil { + return params, err + } + if nRead != uintptr(SizeOfRtlUserProcessParameters) { + return params, errors.Errorf("RTL_USER_PROCESS_PARAMETERS: short read (%d/%d)", nRead, SizeOfRtlUserProcessParameters) + } + + params = *(*RtlUserProcessParameters)(unsafe.Pointer(¶msBuf[0])) + return params, nil +} + +func ReadProcessUnicodeString(handle syscall.Handle, s *UnicodeString) ([]byte, error) { + buf := make([]byte, s.Size) + nRead, err := ReadProcessMemory(handle, s.Buffer, buf) + if err != nil { + return nil, err + } + if nRead != uintptr(s.Size) { + return nil, errors.Errorf("unicode string: short read: (%d/%d)", nRead, s.Size) + } + return buf, nil +} + +// Use Windows' CommandLineToArgv API to split an UTF-16 command line string +// into a list of parameters. +func ByteSliceToStringSlice(utf16 []byte) ([]string, error) { + if len(utf16) == 0 { + return nil, nil + } + var numArgs int32 + argsWide, err := syscall.CommandLineToArgv((*uint16)(unsafe.Pointer(&utf16[0])), &numArgs) + if err != nil { + return nil, err + } + args := make([]string, numArgs) + for idx := range args { + args[idx] = syscall.UTF16ToString(argsWide[idx][:]) + } + return args, nil +} + +// ReadProcessMemory reads from another process memory. The Handle needs to have +// the PROCESS_VM_READ right. +// A zero-byte read is a no-op, no error is returned. +func ReadProcessMemory(handle syscall.Handle, baseAddress uintptr, dest []byte) (numRead uintptr, err error) { + n := len(dest) + if n == 0 { + return 0, nil + } + if err = _ReadProcessMemory(handle, baseAddress, uintptr(unsafe.Pointer(&dest[0])), uintptr(n), &numRead); err != nil { + return 0, err + } + return numRead, nil +} + +func GetTickCount64() (uptime uint64, err error) { + if uptime, err = _GetTickCount64(); err != nil { + return 0, err + } + return uptime, nil +} + // Use "GOOS=windows go generate -v -x ." to generate the source. // Add -trace to enable debug prints around syscalls. @@ -467,3 +581,5 @@ func UTF16SliceToStringSlice(buffer []uint16) []string { //sys _FindNextVolume(handle syscall.Handle, volumeName *uint16, size uint32) (err error) = kernel32.FindNextVolumeW //sys _FindVolumeClose(handle syscall.Handle) (err error) = kernel32.FindVolumeClose //sys _GetVolumePathNamesForVolumeName(volumeName string, buffer *uint16, bufferSize uint32, length *uint32) (err error) = kernel32.GetVolumePathNamesForVolumeNameW +//sys _ReadProcessMemory(handle syscall.Handle, baseAddress uintptr, buffer uintptr, size uintptr, numRead *uintptr) (err error) = kernel32.ReadProcessMemory +//sys _GetTickCount64() (uptime uint64, err error) = kernel32.GetTickCount64 diff --git a/sys/windows/syscall_windows_test.go b/sys/windows/syscall_windows_test.go index 96e4022b7..804da9232 100644 --- a/sys/windows/syscall_windows_test.go +++ b/sys/windows/syscall_windows_test.go @@ -181,3 +181,48 @@ func TestNtQueryProcessBasicInformation(t *testing.T) { t.Logf("NtQueryProcessBasicInformation: %+v", info) } + +func TestGetTickCount64(t *testing.T) { + uptime, err := GetTickCount64() + if err != nil { + t.Fatal(err) + } + assert.NotZero(t, uptime) +} + +func TestGetUserProcessParams(t *testing.T) { + h, err := syscall.OpenProcess(syscall.PROCESS_QUERY_INFORMATION|PROCESS_VM_READ, false, uint32(syscall.Getpid())) + if err != nil { + t.Fatal(err) + } + info, err := NtQueryProcessBasicInformation(h) + if err != nil { + t.Fatal(err) + } + userProc, err := GetUserProcessParams(h, info) + if err != nil { + t.Fatal(err) + } + assert.EqualValues(t, os.Getpid(), info.UniqueProcessID) + assert.EqualValues(t, os.Getppid(), info.InheritedFromUniqueProcessID) + assert.NotEmpty(t, userProc.CommandLine) +} +func TestReadProcessUnicodeString(t *testing.T) { + h, err := syscall.OpenProcess(syscall.PROCESS_QUERY_INFORMATION|PROCESS_VM_READ, false, uint32(syscall.Getpid())) + if err != nil { + t.Fatal(err) + } + info, err := NtQueryProcessBasicInformation(h) + if err != nil { + t.Fatal(err) + } + userProc, err := GetUserProcessParams(h, info) + if err != nil { + t.Fatal(err) + } + read, err := ReadProcessUnicodeString(h, &userProc.CommandLine) + if err != nil { + t.Fatal(err) + } + assert.NotEmpty(t, read) +} diff --git a/sys/windows/zsyscall_windows.go b/sys/windows/zsyscall_windows.go index 8da079aba..cd5d9ca32 100644 --- a/sys/windows/zsyscall_windows.go +++ b/sys/windows/zsyscall_windows.go @@ -1,4 +1,4 @@ -// MACHINE GENERATED BY 'go generate' COMMAND; DO NOT EDIT +// Code generated by 'go generate'; DO NOT EDIT. package windows @@ -60,6 +60,8 @@ var ( procFindNextVolumeW = modkernel32.NewProc("FindNextVolumeW") procFindVolumeClose = modkernel32.NewProc("FindVolumeClose") procGetVolumePathNamesForVolumeNameW = modkernel32.NewProc("GetVolumePathNamesForVolumeNameW") + procReadProcessMemory = modkernel32.NewProc("ReadProcessMemory") + procGetTickCount64 = modkernel32.NewProc("GetTickCount64") ) func _GlobalMemoryStatusEx(buffer *MemoryStatusEx) (err error) { @@ -347,3 +349,28 @@ func __GetVolumePathNamesForVolumeName(volumeName *uint16, buffer *uint16, buffe } return } + +func _ReadProcessMemory(handle syscall.Handle, baseAddress uintptr, buffer uintptr, size uintptr, numRead *uintptr) (err error) { + r1, _, e1 := syscall.Syscall6(procReadProcessMemory.Addr(), 5, uintptr(handle), uintptr(baseAddress), uintptr(buffer), uintptr(size), uintptr(unsafe.Pointer(numRead)), 0) + if r1 == 0 { + if e1 != 0 { + err = errnoErr(e1) + } else { + err = syscall.EINVAL + } + } + return +} + +func _GetTickCount64() (uptime uint64, err error) { + r0, _, e1 := syscall.Syscall(procGetTickCount64.Addr(), 0, 0, 0, 0) + uptime = uint64(r0) + if uptime == 0 { + if e1 != 0 { + err = errnoErr(e1) + } else { + err = syscall.EINVAL + } + } + return +} From b204596e0c134a698ef4c9a2da29bfaa0d22be1e Mon Sep 17 00:00:00 2001 From: Mariana Date: Fri, 19 Apr 2019 10:28:21 +0200 Subject: [PATCH 02/10] Add more tests --- sigar_windows.go | 2 +- sys/windows/syscall_windows_test.go | 33 +++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/sigar_windows.go b/sigar_windows.go index 3a5ba2a9b..34602169b 100644 --- a/sigar_windows.go +++ b/sigar_windows.go @@ -376,7 +376,6 @@ func (self *ProcArgs) Get(pid int) error { return errors.Wrapf(err, "OpenProcess failed for pid=%v", pid) } defer syscall.CloseHandle(handle) - var args []string pbi, err := windows.NtQueryProcessBasicInformation(handle) if err != nil { return errors.Wrapf(err, "NtQueryProcessBasicInformation failed for pid=%v", pid) @@ -388,6 +387,7 @@ func (self *ProcArgs) Get(pid int) error { if err != nil { return nil } + var args []string if argsW, err := windows.ReadProcessUnicodeString(handle, &userProcParams.CommandLine); err == nil { args, err = windows.ByteSliceToStringSlice(argsW) if err != nil { diff --git a/sys/windows/syscall_windows_test.go b/sys/windows/syscall_windows_test.go index 804da9232..70ae564fa 100644 --- a/sys/windows/syscall_windows_test.go +++ b/sys/windows/syscall_windows_test.go @@ -1,10 +1,12 @@ package windows import ( + "encoding/binary" "os" "runtime" "syscall" "testing" + "unsafe" "github.com/pkg/errors" "github.com/stretchr/testify/assert" @@ -203,6 +205,7 @@ func TestGetUserProcessParams(t *testing.T) { if err != nil { t.Fatal(err) } + assert.NoError(t, err) assert.EqualValues(t, os.Getpid(), info.UniqueProcessID) assert.EqualValues(t, os.Getppid(), info.InheritedFromUniqueProcessID) assert.NotEmpty(t, userProc.CommandLine) @@ -224,5 +227,35 @@ func TestReadProcessUnicodeString(t *testing.T) { if err != nil { t.Fatal(err) } + assert.NoError(t, err) assert.NotEmpty(t, read) } + +func TestByteSliceToStringSlice(t *testing.T) { + cmd := syscall.GetCommandLine() + b := make([]byte, unsafe.Sizeof(cmd)) + binary.LittleEndian.PutUint16(b, *cmd) + hes, err := ByteSliceToStringSlice(b) + assert.NoError(t, err) + assert.NotEmpty(t, hes) +} + +func TestReadProcessMemory(t *testing.T) { + h, err := syscall.OpenProcess(syscall.PROCESS_QUERY_INFORMATION|PROCESS_VM_READ, false, uint32(syscall.Getpid())) + if err != nil { + t.Fatal(err) + } + info, err := NtQueryProcessBasicInformation(h) + if err != nil { + t.Fatal(err) + } + pebSize := 0x20 + 8 + if unsafe.Sizeof(uintptr(0)) == 4 { + pebSize = 0x10 + 8 + } + peb := make([]byte, pebSize) + nRead, err := ReadProcessMemory(h, info.PebBaseAddress, peb) + assert.NoError(t, err) + assert.NotEmpty(t, nRead) + assert.EqualValues(t, nRead, uintptr(pebSize)) +} From 168704054da70f385de69ee40d2b0143a7c3c1a9 Mon Sep 17 00:00:00 2001 From: Mariana Date: Fri, 19 Apr 2019 15:34:32 +0200 Subject: [PATCH 03/10] Add more tests for coverage --- sys/windows/syscall_windows_test.go | 32 +++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/sys/windows/syscall_windows_test.go b/sys/windows/syscall_windows_test.go index 70ae564fa..5d636e7f9 100644 --- a/sys/windows/syscall_windows_test.go +++ b/sys/windows/syscall_windows_test.go @@ -259,3 +259,35 @@ func TestReadProcessMemory(t *testing.T) { assert.NotEmpty(t, nRead) assert.EqualValues(t, nRead, uintptr(pebSize)) } +func TestGetAccessPaths(t *testing.T) { + paths, err := GetAccessPaths() + if err != nil { + t.Fatal(err) + } + assert.NotEmpty(t, paths) + assert.True(t, len(paths) >= 1) +} + +func TestGetVolumes(t *testing.T) { + paths, err := GetVolumes() + if err != nil { + t.Fatal(err) + } + assert.NotEmpty(t, paths) + assert.True(t, len(paths) >= 1) +} + +func TestGetVolumePathsForVolume(t *testing.T) { + volumes, err := GetVolumes() + if err != nil { + t.Fatal(err) + } + assert.NotNil(t, volumes) + assert.True(t, len(volumes) >= 1) + volumePath, err := GetVolumePathsForVolume(volumes[0]) + if err != nil { + t.Fatal(err) + } + assert.NotNil(t, volumePath) + assert.True(t, len(volumePath) >= 1) +} From 106efba697f2f96df1f15a8951e3922d0ea25611 Mon Sep 17 00:00:00 2001 From: Mariana Date: Fri, 19 Apr 2019 16:24:45 +0200 Subject: [PATCH 04/10] Add tests --- sys/windows/syscall_windows_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/sys/windows/syscall_windows_test.go b/sys/windows/syscall_windows_test.go index 5d636e7f9..40a4ed530 100644 --- a/sys/windows/syscall_windows_test.go +++ b/sys/windows/syscall_windows_test.go @@ -240,6 +240,13 @@ func TestByteSliceToStringSlice(t *testing.T) { assert.NotEmpty(t, hes) } +func TestByteSliceToStringSliceEmptyBytes(t *testing.T) { + b := make([]byte, 0) + hes, err := ByteSliceToStringSlice(b) + assert.NoError(t, err) + assert.Empty(t, hes) +} + func TestReadProcessMemory(t *testing.T) { h, err := syscall.OpenProcess(syscall.PROCESS_QUERY_INFORMATION|PROCESS_VM_READ, false, uint32(syscall.Getpid())) if err != nil { @@ -259,6 +266,27 @@ func TestReadProcessMemory(t *testing.T) { assert.NotEmpty(t, nRead) assert.EqualValues(t, nRead, uintptr(pebSize)) } + +// A zero-byte read is a no-op, no error is returned. +func TestReadProcessMemoryZeroByteRead(t *testing.T) { + peb := make([]byte, 0) + var h syscall.Handle + var address uintptr + nRead, err := ReadProcessMemory(h, address, peb) + assert.NoError(t, err) + assert.Empty(t, nRead) +} + +func TestReadProcessMemoryInvalidHandler(t *testing.T) { + peb := make([]byte, 10) + var h syscall.Handle + var address uintptr + nRead, err := ReadProcessMemory(h, address, peb) + assert.Error(t, err) + assert.EqualValues(t, err.Error(), "The handle is invalid.") + assert.Empty(t, nRead) +} + func TestGetAccessPaths(t *testing.T) { paths, err := GetAccessPaths() if err != nil { From 6a01c7b2c2ab39b5be30625c2b8f43b69cfe2aab Mon Sep 17 00:00:00 2001 From: Mariana Date: Fri, 19 Apr 2019 17:02:22 +0200 Subject: [PATCH 05/10] Add tests for coverage --- sigar_windows.go | 2 +- sys/windows/syscall_windows_test.go | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/sigar_windows.go b/sigar_windows.go index 34602169b..490a5cb45 100644 --- a/sigar_windows.go +++ b/sigar_windows.go @@ -84,7 +84,7 @@ func (self *Uptime) Get() error { if bootTime == nil { uptime, err := windows.GetTickCount64() if err != nil { - return errors.Wrap(err, "failed to get boot time using WMI") + return errors.Wrap(err, "failed to get boot time using win32 api") } var boot = time.Unix(int64(uptime), 0) bootTime = &boot diff --git a/sys/windows/syscall_windows_test.go b/sys/windows/syscall_windows_test.go index 40a4ed530..6607e8897 100644 --- a/sys/windows/syscall_windows_test.go +++ b/sys/windows/syscall_windows_test.go @@ -210,6 +210,15 @@ func TestGetUserProcessParams(t *testing.T) { assert.EqualValues(t, os.Getppid(), info.InheritedFromUniqueProcessID) assert.NotEmpty(t, userProc.CommandLine) } + +func TestGetUserProcessParamsInvalidHandle(t *testing.T) { + var handle syscall.Handle + var info = ProcessBasicInformation{PebBaseAddress: uintptr(0)} + userProc, err := GetUserProcessParams(handle, info) + assert.EqualValues(t, err.Error(), "The handle is invalid.") + assert.Empty(t, userProc) +} + func TestReadProcessUnicodeString(t *testing.T) { h, err := syscall.OpenProcess(syscall.PROCESS_QUERY_INFORMATION|PROCESS_VM_READ, false, uint32(syscall.Getpid())) if err != nil { @@ -230,6 +239,13 @@ func TestReadProcessUnicodeString(t *testing.T) { assert.NoError(t, err) assert.NotEmpty(t, read) } +func TestReadProcessUnicodeStringInvalidHandle(t *testing.T) { + var handle syscall.Handle + var cmd = UnicodeString{Size: 5, MaximumLength: 400, Buffer: 400} + read, err := ReadProcessUnicodeString(handle, &cmd) + assert.EqualValues(t, err.Error(), "The handle is invalid.") + assert.Empty(t, read) +} func TestByteSliceToStringSlice(t *testing.T) { cmd := syscall.GetCommandLine() From c8fd3ef63b4384dc3bc63609735317966a3d50ad Mon Sep 17 00:00:00 2001 From: Mariana Date: Tue, 23 Apr 2019 09:42:31 +0200 Subject: [PATCH 06/10] Remove WIn32Process related structs --- sigar_interface_test.go | 7 ++++--- sigar_windows.go | 24 ++---------------------- sys/windows/syscall_windows_test.go | 4 ++-- 3 files changed, 8 insertions(+), 27 deletions(-) diff --git a/sigar_interface_test.go b/sigar_interface_test.go index 8545484c5..793d0246b 100644 --- a/sigar_interface_test.go +++ b/sigar_interface_test.go @@ -123,10 +123,11 @@ func TestProcTime(t *testing.T) { } func TestProcArgs(t *testing.T) { - args := ProcArgs{} - if assert.NoError(t, args.Get(os.Getppid())) { - assert.NotEmpty(t, args.List) + procArgs := ProcArgs{} + if assert.NoError(t, procArgs.Get(os.Getppid())) { + assert.NotEmpty(t, procArgs.List) } + assert.Error(t, procArgs.Get(invalidPid)) } func TestProcEnv(t *testing.T) { diff --git a/sigar_windows.go b/sigar_windows.go index 490a5cb45..66f294e9c 100644 --- a/sigar_windows.go +++ b/sigar_windows.go @@ -16,21 +16,6 @@ import ( "github.com/pkg/errors" ) -// Win32_Process represents a process on the Windows operating system. If -// additional fields are added here (that match the Windows struct) they will -// automatically be populated when calling getWin32Process. -// https://msdn.microsoft.com/en-us/library/windows/desktop/aa394372(v=vs.85).aspx -type Win32_Process struct { - CommandLine *string -} - -// Win32_OperatingSystem WMI class represents a Windows-based operating system -// installed on a computer. -// https://msdn.microsoft.com/en-us/library/windows/desktop/aa394239(v=vs.85).aspx -type Win32_OperatingSystem struct { - LastBootUpTime time.Time -} - var ( // version is Windows version of the host OS. version = windows.GetWindowsVersion() @@ -387,17 +372,12 @@ func (self *ProcArgs) Get(pid int) error { if err != nil { return nil } - var args []string if argsW, err := windows.ReadProcessUnicodeString(handle, &userProcParams.CommandLine); err == nil { - args, err = windows.ByteSliceToStringSlice(argsW) + self.List, err = windows.ByteSliceToStringSlice(argsW) if err != nil { - args = nil + return err } } - var process = Win32_Process{CommandLine: &args[0]} - if process.CommandLine != nil { - self.List = []string{*process.CommandLine} - } return nil } diff --git a/sys/windows/syscall_windows_test.go b/sys/windows/syscall_windows_test.go index 6607e8897..d6f92c808 100644 --- a/sys/windows/syscall_windows_test.go +++ b/sys/windows/syscall_windows_test.go @@ -258,9 +258,9 @@ func TestByteSliceToStringSlice(t *testing.T) { func TestByteSliceToStringSliceEmptyBytes(t *testing.T) { b := make([]byte, 0) - hes, err := ByteSliceToStringSlice(b) + cmd, err := ByteSliceToStringSlice(b) assert.NoError(t, err) - assert.Empty(t, hes) + assert.Empty(t, cmd) } func TestReadProcessMemory(t *testing.T) { From a4e7d68d0ceefe21c9ff4662f7c12896cd9b5c2b Mon Sep 17 00:00:00 2001 From: Mariana Date: Tue, 23 Apr 2019 10:20:41 +0200 Subject: [PATCH 07/10] Fix test failing on osx --- CHANGELOG.md | 1 + sigar_interface_test.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cdfc5627..2d245521c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - Added missing runtime import for FreeBSD. #104 - Handle nil command line in Windows processes. #110 +- Replaced the WMI queries with win32 apis due to high CPU usage. #11840 ## [0.9.0] diff --git a/sigar_interface_test.go b/sigar_interface_test.go index 793d0246b..fbc561531 100644 --- a/sigar_interface_test.go +++ b/sigar_interface_test.go @@ -127,7 +127,7 @@ func TestProcArgs(t *testing.T) { if assert.NoError(t, procArgs.Get(os.Getppid())) { assert.NotEmpty(t, procArgs.List) } - assert.Error(t, procArgs.Get(invalidPid)) + assert.Error(t, skipNotImplemented(t, procArgs.Get(invalidPid), "darwin")) } func TestProcEnv(t *testing.T) { From 050f7b2905af8c7da3ef59819741c9e63f96d3f6 Mon Sep 17 00:00:00 2001 From: Mariana Date: Tue, 23 Apr 2019 10:34:11 +0200 Subject: [PATCH 08/10] Work on tests --- sigar_interface_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/sigar_interface_test.go b/sigar_interface_test.go index fbc561531..abbb0fdcf 100644 --- a/sigar_interface_test.go +++ b/sigar_interface_test.go @@ -127,7 +127,6 @@ func TestProcArgs(t *testing.T) { if assert.NoError(t, procArgs.Get(os.Getppid())) { assert.NotEmpty(t, procArgs.List) } - assert.Error(t, skipNotImplemented(t, procArgs.Get(invalidPid), "darwin")) } func TestProcEnv(t *testing.T) { From f932a644f3db6152f4a19af4a32d45c3a91655e7 Mon Sep 17 00:00:00 2001 From: Mariana Date: Tue, 23 Apr 2019 14:45:11 +0200 Subject: [PATCH 09/10] Clean up tests --- sigar_interface_test.go | 4 +++- sys/windows/syscall_windows.go | 9 ++++----- sys/windows/syscall_windows_test.go | 6 ++++-- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/sigar_interface_test.go b/sigar_interface_test.go index abbb0fdcf..cc55281f7 100644 --- a/sigar_interface_test.go +++ b/sigar_interface_test.go @@ -100,7 +100,6 @@ func TestProcState(t *testing.T) { assert.Equal(t, u.Username, state.Username) assert.True(t, state.Ppid > 0, "ppid=%v is non-positive", state.Ppid) } - assert.Error(t, state.Get(invalidPid)) } @@ -127,6 +126,9 @@ func TestProcArgs(t *testing.T) { if assert.NoError(t, procArgs.Get(os.Getppid())) { assert.NotEmpty(t, procArgs.List) } + if runtime.GOOS != "darwin" { + assert.Error(t, procArgs.Get(invalidPid)) + } } func TestProcEnv(t *testing.T) { diff --git a/sys/windows/syscall_windows.go b/sys/windows/syscall_windows.go index d1222d29c..655836701 100644 --- a/sys/windows/syscall_windows.go +++ b/sys/windows/syscall_windows.go @@ -22,11 +22,10 @@ const ( PROCESS_QUERY_LIMITED_INFORMATION uint32 = 0x1000 PROCESS_VM_READ uint32 = 0x0010 ) -const ( - // SizeOfRtlUserProcessParameters gives the size - // of the RtlUserProcessParameters struct. - SizeOfRtlUserProcessParameters = unsafe.Sizeof(RtlUserProcessParameters{}) -) + +// SizeOfRtlUserProcessParameters gives the size +// of the RtlUserProcessParameters struct. +const SizeOfRtlUserProcessParameters = unsafe.Sizeof(RtlUserProcessParameters{}) // MAX_PATH is the maximum length for a path in Windows. // https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx diff --git a/sys/windows/syscall_windows_test.go b/sys/windows/syscall_windows_test.go index d6f92c808..6178e4af7 100644 --- a/sys/windows/syscall_windows_test.go +++ b/sys/windows/syscall_windows_test.go @@ -31,7 +31,6 @@ func TestGetProcessMemoryInfo(t *testing.T) { if err != nil { t.Fatal(err) } - counters, err := GetProcessMemoryInfo(h) if err != nil { t.Fatal(err) @@ -177,7 +176,7 @@ func TestNtQueryProcessBasicInformation(t *testing.T) { if err != nil { t.Fatal(err) } - + defer syscall.CloseHandle(h) assert.EqualValues(t, os.Getpid(), info.UniqueProcessID) assert.EqualValues(t, os.Getppid(), info.InheritedFromUniqueProcessID) @@ -205,6 +204,7 @@ func TestGetUserProcessParams(t *testing.T) { if err != nil { t.Fatal(err) } + defer syscall.CloseHandle(h) assert.NoError(t, err) assert.EqualValues(t, os.Getpid(), info.UniqueProcessID) assert.EqualValues(t, os.Getppid(), info.InheritedFromUniqueProcessID) @@ -236,6 +236,7 @@ func TestReadProcessUnicodeString(t *testing.T) { if err != nil { t.Fatal(err) } + defer syscall.CloseHandle(h) assert.NoError(t, err) assert.NotEmpty(t, read) } @@ -276,6 +277,7 @@ func TestReadProcessMemory(t *testing.T) { if unsafe.Sizeof(uintptr(0)) == 4 { pebSize = 0x10 + 8 } + defer syscall.CloseHandle(h) peb := make([]byte, pebSize) nRead, err := ReadProcessMemory(h, info.PebBaseAddress, peb) assert.NoError(t, err) From 14e4bb4dd37b64243f6bc241c1dbda2fb1973b81 Mon Sep 17 00:00:00 2001 From: Mariana Date: Wed, 24 Apr 2019 13:21:14 +0200 Subject: [PATCH 10/10] Fix on changelog file --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d245521c..196b1a939 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Added ### Fixed +- Replaced the WMI queries with win32 apis due to high CPU usage. #11840 ### Changed @@ -20,7 +21,6 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - Added missing runtime import for FreeBSD. #104 - Handle nil command line in Windows processes. #110 -- Replaced the WMI queries with win32 apis due to high CPU usage. #11840 ## [0.9.0]