From 331c29f8e7ad3301c5b8809abe60dc7751e47b80 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Thu, 12 Sep 2024 07:48:01 -0700 Subject: [PATCH 1/2] remove service and result release calls in wmi --- ee/wmi/wmi.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ee/wmi/wmi.go b/ee/wmi/wmi.go index b46a5edb8..c0e67c352 100644 --- a/ee/wmi/wmi.go +++ b/ee/wmi/wmi.go @@ -153,8 +153,10 @@ func Query(ctx context.Context, slogger *slog.Logger, className string, properti } defer serviceRaw.Clear() + // 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 service := serviceRaw.ToIDispatch() - defer service.Release() slogger.Log(ctx, slog.LevelDebug, "running WMI query", @@ -168,8 +170,10 @@ func Query(ctx context.Context, slogger *slog.Logger, className string, properti } defer resultRaw.Clear() + // the memory of result is released by `defer resultRaw.Clear()` above, + // on windows arm64 machines, calling `resultRaw.Clear()` after `result.Release()` + // would cause a panic result := resultRaw.ToIDispatch() - defer result.Release() if err := oleutil.ForEach(result, handler.HandleVariant); err != nil { return nil, fmt.Errorf("ole foreach: %w", err) From 664b39e7de538215394527d5187acf43c1e1be0e Mon Sep 17 00:00:00 2001 From: James Pickett Date: Thu, 12 Sep 2024 08:38:19 -0700 Subject: [PATCH 2/2] better comment --- ee/wmi/wmi.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/ee/wmi/wmi.go b/ee/wmi/wmi.go index c0e67c352..8caae577a 100644 --- a/ee/wmi/wmi.go +++ b/ee/wmi/wmi.go @@ -153,9 +153,16 @@ func Query(ctx context.Context, slogger *slog.Logger, className string, properti } defer serviceRaw.Clear() - // 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. + // + // Looking at the `serviceRaw.ToIDispatch()` implementation, it's just a cast that returns + // a pointer to the same memory. Which would explain why calling `serviceRaw.Release()` after + // `service.Clear()` causes a panic. It's unclear why this causes a panic on arm64 machines and + // not on amd64 machines. + // + // This also applies to the `resultRaw` and `results` variables below. service := serviceRaw.ToIDispatch() slogger.Log(ctx, slog.LevelDebug, @@ -170,9 +177,7 @@ func Query(ctx context.Context, slogger *slog.Logger, className string, properti } defer resultRaw.Clear() - // the memory of result is released by `defer resultRaw.Clear()` above, - // on windows arm64 machines, calling `resultRaw.Clear()` after `result.Release()` - // would cause a panic + // see above comment about `service.Release()` to explain why `result.Release()` isn't called result := resultRaw.ToIDispatch() if err := oleutil.ForEach(result, handler.HandleVariant); err != nil {