Gosigar's WMI/COM library is leaking memory under Windows

[adriansr]

Gosigar internally uses WMI to fetch some process information under Windows. The use of WMI requires initializing the COM library (CoInitializeEx) on each system thread that is going to use it.

Currently, this happens inside github.com/StackExchange/wmi which calls CoInitializeEx/CoUninitialize for each WMI query.

However, I've found out that each call to CoInitializeEx leaks some memory:

0:000> !mex.mheap -pa 000000000cb394f0
    address 000000000cb394f0 found in
    _HEAP @ 80000
              HEAP_ENTRY Size Prev Flags            UserPtr UserSize - state
        000000000cb394f0 000b 0000  [00]   000000000cb39520    00088 - (busy)
          combase!CClassCache::CClassEntry::`vftable'
        7ffebbc20543 ntdll!RtlpCallInterceptRoutine+0x000000000000003f
        7ffebbbbeee2 ntdll!RtlpAllocateHeapInternal+0x0000000000001142
        7ffeb940de56 combase!CClassCache::CClassEntry::CreateIncomplete+0x0000000000000052
        7ffeb940dd5f combase!CClassCache::CClassEntry::Create+0x0000000000000033
        7ffeb9460932 combase!CClassCache::GetClassObjectActivator+0x00000000000008c2
        7ffeb9454759 combase!CClassCache::GetClassObject+0x0000000000000045
        7ffeb94566f0 combase!ICoGetClassObject+0x00000000000002d0
        7ffeb9436161 combase!CStdMarshal::GetPSFactory+0x00000000000001b1
        7ffeb94352df combase!CStdMarshal::CreateStub+0x000000000000011f
        7ffeb9439b8b combase!CStdMarshal::ConnectSrvIPIDEntry+0x000000000000002f
        7ffeb943abe6 combase!CStdMarshal::MarshalServerIPID+0x0000000000000082
        7ffeb941648e combase!CRemoteUnknown::RemQueryInterface+0x000000000000036e
        7ffeb9728263 rpcrt4!Invoke+0x0000000000000073
        7ffeb978bc0d rpcrt4!Ndr64StubWorker+0x0000000000000bfd
        7ffeb96b68f9 rpcrt4!NdrStubCall3+0x00000000000000c9
        7ffeb93d12c9 combase!CStdStubBuffer_Invoke+0x0000000000000059
        7ffeb941ceb3 combase!ObjectMethodExceptionHandlingAction >+0x0000000000000053
        7ffeb941cb62 combase!DefaultStubInvoke+0x0000000000000222
        7ffeb941b9e8 combase!ServerCall::ContextInvoke+0x0000000000000448
        7ffeb941a65f combase!AppInvoke+0x0000000000000a0f
        7ffeb941965b combase!ComInvokeWithLockAndIPID+0x00000000000009ab
        7ffeb9416af6 combase!ThreadDispatch+0x0000000000000306
        7ffeb9413fba combase!ThreadWndProc+0x00000000000001aa
        7ffeba071c84 user32!UserCallWinProcCheckWow+0x0000000000000274
        7ffeba0715cc user32!DispatchMessageWorker+0x00000000000001ac
        7ffeb940ad0e combase!CDllHost::STAWorkerLoop+0x000000000000009a
        7ffeb940ae43 combase!CDllHost::WorkerThread+0x00000000000000b7
        7ffeb93f8f8b combase!CRpcThread::WorkerLoop+0x00000000000001d3
        7ffeb9497e2c combase!CRpcThreadCache::RpcWorkerThreadEntry+0x000000000000007c
        7ffebbae84d4 kernel32!BaseThreadInitThunk+0x0000000000000014
        7ffebbbfe851 ntdll!RtlUserThreadStart+0x0000000000000021

The leak stopped by performing this initialization only once per thread:

diff --git a/vendor/github.com/StackExchange/wmi/wmi.go b/vendor/github.com/StackExchange/wmi/wmi.go
index 9c688b038..a189721ee 100644
--- a/vendor/github.com/StackExchange/wmi/wmi.go
+++ b/vendor/github.com/StackExchange/wmi/wmi.go
@@ -39,6 +39,7 @@ import (

        "github.com/go-ole/go-ole"
        "github.com/go-ole/go-ole/oleutil"
+       "golang.org/x/sys/windows"
 )

 var l = log.New(os.Stdout, "", log.LstdFlags)
@@ -112,6 +113,8 @@ type Client struct {
 // DefaultClient is the default Client and is used by Query, QueryNamespace
 var DefaultClient = &Client{}

+var coInitMap = make(map[uint32]struct{})
+
 // Query runs the WQL query and appends the values to dst.
 //
 // dst must have type *[]S or *[]*S, for some struct type S. Fields selected in
@@ -138,14 +141,17 @@ func (c *Client) Query(query string, dst interface{}, connectServerArgs ...inter
        runtime.LockOSThread()
        defer runtime.UnlockOSThread()

-       err := ole.CoInitializeEx(0, ole.COINIT_MULTITHREADED)
-       if err != nil {
-               oleCode := err.(*ole.OleError).Code()
-               if oleCode != ole.S_OK && oleCode != S_FALSE {
-                       return err
+       id := windows.GetCurrentThreadId()
+       if _, found := coInitMap[id]; !found {
+               err := ole.CoInitializeEx(0, ole.COINIT_MULTITHREADED)
+               if err != nil {
+                       oleCode := err.(*ole.OleError).Code()
+                       if oleCode != ole.S_OK && oleCode != S_FALSE {
+                               return err
+                       }
                }
+               coInitMap[id] = struct{}{}
        }
-       defer ole.CoUninitialize()

However, I'm not 100% sure that code is safe as there is no matching CoUninitialize call anymore.

We should either improve that code or migrate uses of gosigar to go-sysinfo which does not use WMI.

This currently affects Metricbeat's system/process, system/diskio and logging.metrics reporting every 30s.

[narph]

hi @adriansr , this has already been tackled in the PR's
for process: elastic/gosigar#116 (thanks for elastic/gosigar#119)
for diskio: #11635
Anything else missing here?

[adriansr]

Nothing missing, I was looking at outdated branches. My bad.