Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

james/remove wmi unneeded releases #1863

Merged
merged 2 commits into from
Sep 12, 2024

Conversation

James-Pickett
Copy link
Contributor

@James-Pickett James-Pickett commented Sep 12, 2024

I tested this by creating a small program that runs in a loop only clearing the rawResult and rawService. After 3K iterations, memory rose to around 9MB and remained stable. After 50K iterations memory was still at 9MB. This test was performed on both arm64 and amd64.

ee/wmi/wmi.go Outdated
Comment on lines 156 to 158
// the memory of result is released by `defer serviceRaw.Clear()` above,
// on windows arm64 machines, calling `service.Clear()` after `serviceRaw.Release()`
// would cause a panic
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// the memory of result is released by `defer serviceRaw.Clear()` above,
// on windows arm64 machines, calling `service.Clear()` after `serviceRaw.Release()`
// would cause a panic
// In testing, we find we do not need to `service.Release(). The memory of result is released
// by `defer serviceRaw.Clear()` above, furthermore on windows arm64 machines, calling
// `service.Clear()` after `serviceRaw.Release()` causes a panic

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or something. I think there are probably better phrases. More imperative or something

@James-Pickett James-Pickett marked this pull request as ready for review September 12, 2024 15:39
@James-Pickett James-Pickett added this pull request to the merge queue Sep 12, 2024
Merged via the queue into main with commit 70114e3 Sep 12, 2024
29 checks passed
@James-Pickett James-Pickett deleted the james/remove-wmi-unneeded-releases branch September 12, 2024 18:20
RebeccaMahany added a commit to RebeccaMahany/launcher that referenced this pull request Sep 13, 2024
desktop triggers Windows Hello

Fix timeout, small refactor

Tidy up names, add documentation

Retrieve key credential status

Retrieve pubkey

Get attestation

windows arm64 fixes, upgrade winio and thrift (kolide#1858)

Fix `autoupdate_managed` table value for MacOS 15 (kolide#1862)

james/remove wmi unneeded releases (kolide#1863)

Check windows service manager settings prior to setting them (kolide#1859)

Co-authored-by: Michael <60191460+lurky@users.noreply.github.com>
Co-authored-by: seph <seph@kolide.co>
Co-authored-by: Rebecca Mahany-Horton <rebeccamahany@gmail.com>

Update function signature

Move to ee
@RebeccaMahany RebeccaMahany added the bug-fixes Bug Fixes label Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-fixes Bug Fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants