Skip to content

Commit

Permalink
add PssFreeSnapshot (#115)
Browse files Browse the repository at this point in the history
## What does this PR do?

Frees the memory that is allocated when PssCaptureSnapshot is called.

## Why is it important?

memory leak

## Checklist

- [x] My code follows the style guidelines of this project
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] I have added an entry in `CHANGELOG.md`

## Author's Checklist

<!-- Recommended
Add a checklist of things that are required to be reviewed in order to
have the PR approved
-->
- [ ] Written by hand. Need to figure out what needs to happen for "go
generate" to work. Does not work from Mac.

## Related issues

<!-- Recommended
Link related issues below. Insert the issue link or reference after the
word "Closes" if merging this should automatically close it.

- Closes #123
- Relates #123
- Requires #123
- Superseds #123
-->
- 

## Testing

- run metricbeat 8.11.0 or 8.11.1 on Windows, notice memory consumption
of metricbeat continually climbs
- rebuild metricbeat using this pr
- memory consumption rises in first 10min or so, but is steady after
that
  • Loading branch information
leehinman authored Nov 21, 2023
1 parent c117bd8 commit f61b864
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 6 deletions.
25 changes: 19 additions & 6 deletions metric/system/process/process_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,26 +80,39 @@ func GetInfoForPid(_ resolve.Resolver, pid int) (ProcState, error) {
}

func FetchNumThreads(pid int) (int, error) {
pHandle, err := syscall.OpenProcess(
targetProcessHandle, err := syscall.OpenProcess(
xsyswindows.PROCESS_QUERY_INFORMATION,
false,
uint32(pid))
if err != nil {
return 0, fmt.Errorf("OpenProcess failed for PID %d: %w", pid, err)
}
defer syscall.CloseHandle(pHandle)
defer syscall.CloseHandle(targetProcessHandle)

currentProcessHandle, err := syscall.GetCurrentProcess()
if err != nil {
return 0, fmt.Errorf("GetCurrentProcess failed: %w", err)
}
// The pseudo handle need not be closed when it is no longer
// needed, calling CloseHandle has no effect. Adding here to
// remind us to close any handles we open.
defer syscall.CloseHandle(currentProcessHandle)

var snapshotHandle syscall.Handle
err = PssCaptureSnapshot(pHandle, PSSCaptureThreads, 0, &snapshotHandle)
err = PssCaptureSnapshot(targetProcessHandle, PSSCaptureThreads, 0, &snapshotHandle)
if err != nil {
return 0, fmt.Errorf("PssCaptureSnapshot failed: %w", err)
}

info := PssThreadInformation{}
buffSize := unsafe.Sizeof(info)
err = PssQuerySnapshot(snapshotHandle, PssQueryThreadInformation, &info, uint32(buffSize))
if err != nil {
return 0, fmt.Errorf("PssQuerySnapshot failed: %w", err)
queryErr := PssQuerySnapshot(snapshotHandle, PssQueryThreadInformation, &info, uint32(buffSize))
freeErr := PssFreeSnapshot(currentProcessHandle, snapshotHandle)
if queryErr != nil || freeErr != nil {
//Join discards any nil errors
return 0, errors.Join(
fmt.Errorf("PssQuerySnapshot failed: %w", queryErr),
fmt.Errorf("PssFreeSnapshot failed: %w", freeErr))
}

return int(info.ThreadsCaptured), nil
Expand Down
2 changes: 2 additions & 0 deletions metric/system/process/syscall_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package process
// - https://learn.microsoft.com/en-us/previous-versions/windows/desktop/proc_snap/overview-of-process-snapshotting
// PssCaptureSnapshot docs in https://learn.microsoft.com/en-us/windows/win32/api/processsnapshot/nf-processsnapshot-psscapturesnapshot
// PssQuerySnapshot docs in https://learn.microsoft.com/en-us/windows/win32/api/processsnapshot/nf-processsnapshot-pssquerysnapshot
// PssFreeSnapshot docs in https://learn.microsoft.com/en-us/windows/win32/api/processsnapshot/nf-processsnapshot-pssfreesnapshot

// Use golang.org/x/sys/windows/mkwinsyscall instead of adriansr/mksyscall
// below once https://github.com/golang/go/issues/42373 is fixed.
Expand All @@ -29,6 +30,7 @@ package process

//sys PssCaptureSnapshot(processHandle syscall.Handle, captureFlags PSSCaptureFlags, threadContextFlags uint32, snapshotHandle *syscall.Handle) (err error) [failretval!=0] = kernel32.PssCaptureSnapshot
//sys PssQuerySnapshot(snapshotHandle syscall.Handle, informationClass uint32, buffer *PssThreadInformation, bufferLength uint32) (err error) [failretval!=0] = kernel32.PssQuerySnapshot
//sys PssFreeSnapshot(processHandle syscall.Handle, snapshotHandle syscall.Handle) (err error) [failretval!=0] = kernel32.PssFreeSnapshot

// The following constants are PssQueryInformationClass as defined on
// https://learn.microsoft.com/en-us/windows/win32/api/processsnapshot/ne-processsnapshot-pss_query_information_class
Expand Down
9 changes: 9 additions & 0 deletions metric/system/process/zsyscall_windows.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit f61b864

Please sign in to comment.