From b1b2888e0ff19d30a5da826233429bcfd9c4864b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan-Otto=20Kr=C3=B6pke?= Date: Fri, 16 Oct 2026 10:33:18 +0200 Subject: [PATCH] headers: call per-scrape native functions with SyscallN (#2746) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jan-Otto Kröpke --- AGENTS.md | 6 ++- internal/headers/iphlpapi/iphlpapi.go | 7 ++- internal/headers/iphlpapi/iphlpapi_test.go | 8 ++++ internal/headers/psapi/psapi.go | 3 +- internal/headers/psapi/psapi_test.go | 51 ++++++++++++++++++++ internal/headers/winspool/winspool.go | 12 +++-- internal/headers/winspool/winspool_test.go | 8 ++++ internal/headers/wtsapi32/wtsapi32.go | 11 +++-- internal/headers/wtsapi32/wtsapi32_test.go | 54 ++++++++++++++++++++++ 9 files changed, 148 insertions(+), 12 deletions(-) create mode 100644 internal/headers/psapi/psapi_test.go create mode 100644 internal/headers/wtsapi32/wtsapi32_test.go diff --git a/AGENTS.md b/AGENTS.md index 4a676d49..d30e7640 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -57,7 +57,11 @@ license headers. Use structured `slog` attributes and the existing Windows API wrappers (`golang.org/x/sys/windows`, `NewLazySystemDLL`). `LazyProc.Call` panics for a missing export: add every proc a binding calls without its own `Find` check to the package's `lazyproc.Once` list, and call the package's -`Load` in the `Build` of each collector that uses it. Mirrors of native structs +`Load` in the `Build` of each collector that uses it. New calls that run on +every scrape use `syscall.SyscallN(proc.Addr(), ...)` instead of `proc.Call`: +the variadic `Call` moves the values its pointer arguments point to onto the +heap, `SyscallN` keeps them on the stack. Some bindings, e.g. dhcpsapi and +clusapi, still use `Call`. Mirrors of native structs that MI or COM read by offset need `unsafe.Sizeof`/`Offsetof` checks, see [`internal/mi/layout.go`](internal/mi/layout.go). diff --git a/internal/headers/iphlpapi/iphlpapi.go b/internal/headers/iphlpapi/iphlpapi.go index d8f641ec..f8c93351 100644 --- a/internal/headers/iphlpapi/iphlpapi.go +++ b/internal/headers/iphlpapi/iphlpapi.go @@ -20,6 +20,7 @@ package iphlpapi import ( "encoding/binary" "fmt" + "syscall" "unsafe" "github.com/prometheus-community/windows_exporter/internal/headers/lazyproc" @@ -105,7 +106,8 @@ func GetOwnerPIDOfTCPPort(family uint32, tcpPort uint16) (uint32, error) { func getExtendedTcpTable[T any](ulAf uint32, tableClass uint32) ([]T, error) { var size uint32 - ret, _, _ := procGetExtendedTcpTable.Call( + ret, _, _ := syscall.SyscallN( + procGetExtendedTcpTable.Addr(), 0, uintptr(unsafe.Pointer(&size)), 0, @@ -120,7 +122,8 @@ func getExtendedTcpTable[T any](ulAf uint32, tableClass uint32) ([]T, error) { buf := make([]byte, size) - ret, _, _ = procGetExtendedTcpTable.Call( + ret, _, _ = syscall.SyscallN( + procGetExtendedTcpTable.Addr(), uintptr(unsafe.Pointer(&buf[0])), uintptr(unsafe.Pointer(&size)), 0, diff --git a/internal/headers/iphlpapi/iphlpapi_test.go b/internal/headers/iphlpapi/iphlpapi_test.go index 1fbd084c..674af43b 100644 --- a/internal/headers/iphlpapi/iphlpapi_test.go +++ b/internal/headers/iphlpapi/iphlpapi_test.go @@ -54,3 +54,11 @@ func TestGetOwnerPIDOfTCPPort(t *testing.T) { require.NoError(t, err) require.EqualValues(t, os.Getpid(), pid) } + +func BenchmarkGetTCPConnectionStates(b *testing.B) { + b.ReportAllocs() + + for b.Loop() { + _, _ = iphlpapi.GetTCPConnectionStates(windows.AF_INET) + } +} diff --git a/internal/headers/psapi/psapi.go b/internal/headers/psapi/psapi.go index e7faeaa8..873d8645 100644 --- a/internal/headers/psapi/psapi.go +++ b/internal/headers/psapi/psapi.go @@ -18,6 +18,7 @@ package psapi import ( + "syscall" "unsafe" "github.com/prometheus-community/windows_exporter/internal/headers/lazyproc" @@ -61,7 +62,7 @@ func GetPerformanceInfo() (PerformanceInformation, error) { size := uint32(unsafe.Sizeof(lppi)) lppi.cb = size - r1, _, err := procGetPerformanceInfo.Call(uintptr(unsafe.Pointer(&lppi)), uintptr(size)) + r1, _, err := syscall.SyscallN(procGetPerformanceInfo.Addr(), uintptr(unsafe.Pointer(&lppi)), uintptr(size)) if ret := *(*bool)(unsafe.Pointer(&r1)); !ret { return PerformanceInformation{}, err diff --git a/internal/headers/psapi/psapi_test.go b/internal/headers/psapi/psapi_test.go new file mode 100644 index 00000000..74c036cf --- /dev/null +++ b/internal/headers/psapi/psapi_test.go @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Copyright The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build windows + +package psapi_test + +import ( + "testing" + + "github.com/prometheus-community/windows_exporter/internal/headers/psapi" + "github.com/stretchr/testify/require" +) + +func TestGetPerformanceInfo(t *testing.T) { + t.Parallel() + + info, err := psapi.GetPerformanceInfo() + require.NoError(t, err) + require.NotZero(t, info.PageSize) +} + +// TestGetPerformanceInfoDoesNotAllocate checks that the result struct, whose +// address is passed to the native call, stays on the stack. +func TestGetPerformanceInfoDoesNotAllocate(t *testing.T) { + allocs := testing.AllocsPerRun(100, func() { + _, _ = psapi.GetPerformanceInfo() + }) + + require.Zero(t, allocs) +} + +func BenchmarkGetPerformanceInfo(b *testing.B) { + b.ReportAllocs() + + for b.Loop() { + _, _ = psapi.GetPerformanceInfo() + } +} diff --git a/internal/headers/winspool/winspool.go b/internal/headers/winspool/winspool.go index 80e2298c..32dc5eb1 100644 --- a/internal/headers/winspool/winspool.go +++ b/internal/headers/winspool/winspool.go @@ -25,6 +25,7 @@ package winspool import ( "errors" "fmt" + "syscall" "unsafe" "github.com/prometheus-community/windows_exporter/internal/headers/lazyproc" @@ -50,7 +51,8 @@ func Load() error { return load() } // EnumPrinters returns the printers selected by flags, a combination of PRINTER_ENUM_* values. func EnumPrinters(flags uint32) ([]Printer, error) { buf, count, err := enum(func(buf *byte, size uint32, needed, returned *uint32) (uintptr, error) { - r1, _, err := procEnumPrintersW.Call( + r1, _, err := syscall.SyscallN( + procEnumPrintersW.Addr(), uintptr(flags), 0, // local machine 2, // PRINTER_INFO_2 @@ -94,7 +96,8 @@ func OpenPrinter(printerName string) (windows.Handle, error) { var handle windows.Handle - r1, _, err := procOpenPrinterW.Call( + r1, _, err := syscall.SyscallN( + procOpenPrinterW.Addr(), uintptr(unsafe.Pointer(printerNamePtr)), uintptr(unsafe.Pointer(&handle)), 0, // PRINTER_DEFAULTS @@ -107,7 +110,7 @@ func OpenPrinter(printerName string) (windows.Handle, error) { } func ClosePrinter(handle windows.Handle) error { - r1, _, err := procClosePrinter.Call(uintptr(handle)) + r1, _, err := syscall.SyscallN(procClosePrinter.Addr(), uintptr(handle)) if r1 == 0 { return fmt.Errorf("ClosePrinter: %w", err) } @@ -118,7 +121,8 @@ func ClosePrinter(handle windows.Handle) error { // EnumJobs returns all jobs of the printer in queue order. func EnumJobs(handle windows.Handle) ([]Job, error) { buf, count, err := enum(func(buf *byte, size uint32, needed, returned *uint32) (uintptr, error) { - r1, _, err := procEnumJobsW.Call( + r1, _, err := syscall.SyscallN( + procEnumJobsW.Addr(), uintptr(handle), 0, // FirstJob 0xFFFFFFFF, // NoJobs diff --git a/internal/headers/winspool/winspool_test.go b/internal/headers/winspool/winspool_test.go index d3be7dd1..6c323532 100644 --- a/internal/headers/winspool/winspool_test.go +++ b/internal/headers/winspool/winspool_test.go @@ -141,3 +141,11 @@ func TestEnumJobs(t *testing.T) { require.NotEqual(t, -1, idx) require.NotZero(t, printers[idx].Jobs) } + +func BenchmarkEnumPrinters(b *testing.B) { + b.ReportAllocs() + + for b.Loop() { + _, _ = EnumPrinters(PRINTER_ENUM_LOCAL | PRINTER_ENUM_CONNECTIONS) + } +} diff --git a/internal/headers/wtsapi32/wtsapi32.go b/internal/headers/wtsapi32/wtsapi32.go index 856fcef2..87dbca65 100644 --- a/internal/headers/wtsapi32/wtsapi32.go +++ b/internal/headers/wtsapi32/wtsapi32.go @@ -20,6 +20,7 @@ package wtsapi32 import ( "fmt" "log/slog" + "syscall" "unsafe" "github.com/prometheus-community/windows_exporter/internal/headers/lazyproc" @@ -141,7 +142,7 @@ func WTSOpenServer(server string) (windows.Handle, error) { } } - r1, _, err := procWTSOpenServerEx.Call(uintptr(unsafe.Pointer(serverName))) + r1, _, err := syscall.SyscallN(procWTSOpenServerEx.Addr(), uintptr(unsafe.Pointer(serverName))) serverHandle := windows.Handle(r1) if serverHandle == windows.InvalidHandle { @@ -154,11 +155,12 @@ func WTSOpenServer(server string) (windows.Handle, error) { // WTSCloseServer closes a handle opened by WTSOpenServer. The function returns // void, so neither r1 nor the last error say whether it failed. func WTSCloseServer(server windows.Handle) { - _, _, _ = procWTSCloseServer.Call(uintptr(server)) + _, _, _ = syscall.SyscallN(procWTSCloseServer.Addr(), uintptr(server)) } func WTSFreeMemoryEx(class WTSTypeClass, pMemory uintptr, numberOfEntries uint32) error { - r1, _, err := procWTSFreeMemoryEx.Call( + r1, _, err := syscall.SyscallN( + procWTSFreeMemoryEx.Addr(), uintptr(class), pMemory, uintptr(numberOfEntries), @@ -177,7 +179,8 @@ func WTSEnumerateSessionsEx(server windows.Handle, logger *slog.Logger) ([]WTSSe var count uint32 pLevel := uint32(1) - r1, _, err := procWTSEnumerateSessionsEx.Call( + r1, _, err := syscall.SyscallN( + procWTSEnumerateSessionsEx.Addr(), uintptr(server), uintptr(unsafe.Pointer(&pLevel)), uintptr(0), diff --git a/internal/headers/wtsapi32/wtsapi32_test.go b/internal/headers/wtsapi32/wtsapi32_test.go new file mode 100644 index 00000000..21bad402 --- /dev/null +++ b/internal/headers/wtsapi32/wtsapi32_test.go @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Copyright The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build windows + +package wtsapi32_test + +import ( + "log/slog" + "testing" + + "github.com/prometheus-community/windows_exporter/internal/headers/wtsapi32" + "github.com/stretchr/testify/require" +) + +func TestWTSEnumerateSessionsEx(t *testing.T) { + t.Parallel() + + server, err := wtsapi32.WTSOpenServer("") + require.NoError(t, err) + + t.Cleanup(func() { wtsapi32.WTSCloseServer(server) }) + + sessions, err := wtsapi32.WTSEnumerateSessionsEx(server, slog.New(slog.DiscardHandler)) + require.NoError(t, err) + require.NotEmpty(t, sessions, "session 0 always exists") +} + +func BenchmarkWTSEnumerateSessionsEx(b *testing.B) { + server, err := wtsapi32.WTSOpenServer("") + require.NoError(b, err) + + b.Cleanup(func() { wtsapi32.WTSCloseServer(server) }) + + logger := slog.New(slog.DiscardHandler) + + b.ReportAllocs() + + for b.Loop() { + _, _ = wtsapi32.WTSEnumerateSessionsEx(server, logger) + } +}