Bug: Memory leak when using Windows::Management::Deployment::PackageManager

vthib commented 1 year ago

Which crate is this about?

windows

Crate version

0.48.0

Summary

Using the PackageManager::FindPackages() API, and then retrieving HSTRING from the Package method, seems to leak memory.

It took me quite some time to narrow it down, but here is for example a program reproducing the leak:

fn main() {
    let pm = windows::Management::Deployment::PackageManager::new().unwrap();

    loop {
        for package in pm.FindPackages().unwrap() {
            let _name = package.PublisherDisplayName();
        }
    }
}

with:

[dependencies]
windows = { version = "0.48.0", features = [
    "ApplicationModel",
    "Foundation_Collections",
    "Management_Deployment"
]}

This leaks memory quite rapidly:

Removing the retrieval of a HSTRING seems to limit the memory leak.

I'm not sure whether this is a bug in this crate however. I have attempted to reproduce this with a C++/WinRT version, and it seems to exhibit the same leaking behavior:

#include "pch.h"

using namespace winrt;
using namespace Windows::Foundation;
using namespace Windows::Management::Deployment;

int main()
{
    init_apartment();

    PackageManager packageManager;

    for (;;) {
        auto packages = packageManager.FindPackages();

        for (auto const& package : packages) {
            try {
                winrt::hstring name = package.PublisherDisplayName();
            }
            catch (winrt::hresult_error const& ex) {}
        }
    }
}

with pch.h:

#pragma once
#include <winrt/Windows.Foundation.h>
#include <winrt/Windows.Foundation.Collections.h>
#include <winrt/Windows.Management.Deployment.h>
#include <winrt/Windows.ApplicationModel.h>

But i'm not that confident in my C++ nor do i have any experience with winrt, so this code is likely not right. This is why i'm creating this ticket here, I apologize if that's not the right place for it, i'm not sure where to report it otherwise.

kennykerr commented 1 year ago

Thanks for the repros! I tried it myself and indeed that looks like a leak in the API. @jonwis / @DrusTheAxe should know who owns this.

Otherwise, API issues/questions can be reported here: https://docs.microsoft.com/en-us/answers/topics/windows-api.html

DrusTheAxe commented 1 year ago

The C++ code looks fine on its face. If you comment out the .PublisherDisplayName() call does it still leak? If so then it's a PackageManager issue, else it's the Package API (specifically, something in PublisherDisplayName())

Either way it's not Rust, it's a Platform API issue. Can you report this in the WinAppSDK repository for follow up?

riverar commented 1 year ago

@drustheaxe Doesn't leak w/o that call; a stack of a selection of leaked objects below (25926.rs_prerelease). Perhaps related to marshaling around that PackageDownloadProgressServer* or vector of HSTRINGs in Windows::ApplicationModel::PackageServer::GetPackageResourceResolver(Windows::Internal::StateRepository::IPackageResourceResolver * *). Haven't had time to dig any further.

7ffa32cb2878 ntdll!RtlpAllocateHeapInternal+0x000000000009be88
7ffa32c16989 ntdll!RtlAllocateHeap+0x0000000000000059
7ffa30b22c58 combase!CStdMarshal::ConnectCliIPIDEntry+0x0000000000000238
7ffa30b22676 combase!CStdMarshal::MakeCliIPIDEntry+0x000000000000014e
7ffa30b22363 combase!CStdMarshal::UnmarshalIPID+0x000000000000008f
7ffa30b21c15 combase!CStdMarshal::UnmarshalObjRef+0x000000000000029d
7ffa30b218e7 combase!UnmarshalSwitch+0x00000000000000a7
7ffa30b62c25 combase!CoUnmarshalInterface+0x00000000000005a5
7ffa30b61df3 combase!Ndr64ExtInterfacePointerUnmarshall+0x00000000000001e3
7ffa316422f2 RPCRT4!Ndr64pPointerUnmarshallInternal+0x0000000000000252
7ffa3164223e RPCRT4!Ndr64pPointerUnmarshallInternal+0x000000000000019e
7ffa3170cccc RPCRT4!Ndr64pClientUnMarshal+0x000000000000040c
7ffa3170d180 RPCRT4!NdrpClientCall3+0x0000000000000250
7ffa30be9ba8 combase!ObjectStublessClient+0x0000000000000148
7ffa30d186d2 combase!ObjectStubless+0x0000000000000042
7ffa1a918c32 Windows_ApplicationModel!Windows::ApplicationModel::PackageServer::GetPackageResourceResolver+0x00000000000000f2
7ffa1a9188e7 Windows_ApplicationModel!Windows::ApplicationModel::PackageServer::RetrieveMrtString+0x0000000000000137
7ffa1a91852c Windows_ApplicationModel!Windows::ApplicationModel::PackageServer::GetMrtString+0x000000000000004c
7ff6cb69d138 displayname_leak!winrt::impl::consume_Windows_ApplicationModel_IPackage2<winrt::Windows::ApplicationModel::Package>::PublisherDisplayName+0x00000000000000a8
7ff6cb6a0817 displayname_leak!main+0x00000000000000d7
7ff6cb6a3049 displayname_leak!invoke_main+0x0000000000000039
7ff6cb6a2eee displayname_leak!__scrt_common_main_seh+0x000000000000012e
7ff6cb6a2dae displayname_leak!__scrt_common_main+0x000000000000000e
7ff6cb6a30de displayname_leak!mainCRTStartup+0x000000000000000e
7ffa31b11f88 KERNEL32!BaseThreadInitThunk+0x0000000000000018
7ffa32c58241 ntdll!RtlUserThreadStart+0x0000000000000021

vthib commented 1 year ago

Two other things of note:

This is not related only to PublisherDisplayName, I for example have the same leak with InstalledLocation()?.Path(), and I suppose other methods returning hstrings work as well.

I originally run heap snapshots to narrow down the leak, which gave me this stacktrace has always growing:

I'm not sure how useful that is, but I figured I can join it as well.

I will create a new ticket in the WinAppSDK repository: https://github.com/microsoft/WindowsAppSDK/issues/3789

microsoft / windows-rs