microsoft / Windows-Dev-Performance

A repo for developers on Windows to file issues that impede their productivity, efficiency, and efficacy
MIT License
434 stars 20 forks source link

Media Foundation leaks .WEBM, .MKV, and .FLAC file handles #54

Closed ClosetBugSlayer closed 1 day ago

ClosetBugSlayer commented 3 years ago

Windows 10 version 2004 Build 19041.572.

All WEBM, MKV, and FLAC file handles opened with MFCreateSourceReaderFromURL() remain permanently locked by the process even after all interfaces are properly released. This doesn't happen to other file types. This has remained a consistent bug for the past several Windows 10 releases.

TechnikEmpire commented 3 years ago

Seeing this also in 1909 18363.1256

TechnikEmpire commented 3 years ago

Any update on this or suggested workaround?

ClosetBugSlayer commented 3 years ago

I'm beginning to wonder if this was the most effective forum to report this but it doesn't seem like anyone is watching. I don't know where else to go with it, the bug is pretty obvious.

TechnikEmpire commented 3 years ago

Yeah I was a little shocked to have to google my way into this random ticket to find anything about it. It was completely ruining a simple app I made that just transcodes files.

Perhaps report it in developercommunity.visualstudio.com under c++. That seems to be the bin where someone from Microsoft will pick it up and (hopefully) route it to the correct team.

TechnikEmpire commented 3 years ago

@ValZapod Do you have more info to add to your comment? If we can track down this issue in Chromium then we could possibly work around it?

TechnikEmpire commented 3 years ago

@ValZapod .... do you have a link to the patch or? What I'm asking is how did they fix it, because for the rest of us out here running into this windows bug, since Microsoft seems uninterested in fixing it, it would be nice to be able to work around it like others have.

TechnikEmpire commented 3 years ago

@bitcrazed can you shed any light on this issue please?

TechnikEmpire commented 3 years ago

Ok, maybe @mattwojo or @zooba ?

bitcrazed commented 3 years ago

I've sent a mail to find someone who owns this area. Stay tuned. We appreciate your patience.

bitcrazed commented 3 years ago

Hey @TechnikEmpire & @ClosetBugSlayer

I've spoken with some of our engineers re. this issue. They've tried but failed to repro the issue you describe using the following code. Could you try this on your end and see if it works for you?

Alternatively, please share your repro case so our devs can take a look and see if there's anything they can suggest.

    auto startingHandleList = PrintAndReturnProcessHandleList(); // 👈 Base line open handles, no file
    {
        auto coInit = wil::CoInitializeEx(COINIT_MULTITHREADED);
        RETURN_IF_FAILED(MFStartup(MF_VERSION));
        unique_call<decltype(&::MFShutdown), ::MFShutdown> mfshutdown;

        {
            ComPtr<IMFSourceReader> sourceReader;
            RETURN_IF_FAILED(MFCreateSourceReaderFromURL(L"Dichterliebe Op. 48- Im wunderschönen Monat Mai.flac", nullptr, &sourceReader));
            DWORD streamIndex = 0;
            DWORD streamFlags = 0;
            LONGLONG timestamp = 0;
            ComPtr<IMFSample> sample;
            sourceReader->ReadSample(MF_SOURCE_READER_ANY_STREAM, 0, &streamIndex, &streamFlags, &timestamp, &sample);
            auto middleList = PrintAndReturnProcessHandleList(); // 👈 This is while we are reading I see the file handle
            auto midDifference = startingHandleList.Diff(middleList);
            midDifference.PrintHandles();
        }
    }
    auto endingHandleList = PrintAndReturnProcessHandleList(); // 👈 Final list of handles, the file handle is no longer there.
    auto differenceHandleList = startingHandleList.Diff(endingHandleList);
    differenceHandleList.PrintHandles();

    // Note: `PrintAndReturnProcessHandleList()` queries the handles with NtQuerySystemInformationFunction and so should list all of them.
TechnikEmpire commented 3 years ago

Thanks for the update. A puzzle indeed, I'll try to get a repro. However from your given example, I suspect that fully shutting down MF before checking the handles is hiding the issue, but I may be wrong.

bitcrazed commented 3 years ago

@ClosetBugSlayer It's really difficult to help diagnose a problem without a repro case. Please create and share a minimal repro and share so we can take a look.

TechnikEmpire commented 3 years ago

@ClosetBugSlayer Do you have a specific file you were able to repro on? I'm unable so far to reproduce but where I originally observed this was a sort of drag-net, scooping up thousands of files. I'll try to hunt one of those streams down but thought I'd ask in the meantime.

ClosetBugSlayer commented 3 years ago

OK I had it slightly wrong. After testing, MFCreateSourceReaderFromURL() isn't leaking FLAC handles. IMFSourceResolver.CreateObjectFromURL() is where my leak is happening, and I get the interface from MFCreateSourceResolver().

This C# interop chunk from my project doesn't directly compile but it should be very easy to follow:

Win32Api.MFCreateSourceResolver(out IMFSourceResolver sourceResolver).TryThrowError();

sourceResolver.CreateObjectFromURL(
    _strFileSystemPath,
    MediaFoundationSourceResolverFlags.MF_RESOLUTION_MEDIASOURCE
    | MediaFoundationSourceResolverFlags.MF_RESOLUTION_CONTENT_DOES_NOT_HAVE_TO_MATCH_EXTENSION_OR_MIME_TYPE
    | MediaFoundationSourceResolverFlags.MF_RESOLUTION_READ,
    null,
    out MF_OBJECT_TYPE type,
    out IUnknown objMediaSource)
    .TryThrowError()

Marshal.ReleaseComObject(objMediaSource);
Marshal.ReleaseComObject(sourceResolver);
TechnikEmpire commented 3 years ago

@bitcrazed I'm not sure what happened, but I'm no longer able to reproduce this even in the original software where it was first observed. I also created a standalone example with full HW support etc, no repro. Sorry to take up your time for nothing!

TechnikEmpire commented 3 years ago

Should be noted though that I observed this issue with MFCreateSourceReaderFromURL, not MFCreateSourceResolver.

ClosetBugSlayer commented 3 years ago

Should be noted though that I observed this issue with MFCreateSourceReaderFromURL, not MFCreateSourceResolver.

I could have sworn it was MFCreateSourceReaderFromURL but last night changed things. We're in the twilight zone now.

bitcrazed commented 3 years ago

@ClosetBugSlayer & @TechnikEmpire - Thanks for investigating at your end.

Since this may have become a heisenbug with no repro, I'll close this issue for now to get it off our radar. If you ever do manage to create a repro, please feel free to ping this thread, share your repro case, and request that the bug be reopened.

ClosetBugSlayer commented 3 years ago

@ClosetBugSlayer & @TechnikEmpire - Thanks for investigating at your end.

Since this may have become a heisenbug with no repro, I'll close this issue for now to get it off our radar. If you ever do manage to create a repro, please feel free to ping this thread, share your repro case, and request that the bug be reopened.

MediaFoundationLeakBug.zip

TADAAAAAA I attached a repro. I hope the comments in the .CPP file guide you through the repro successfully. Please reopen this bug and crush it mercilessly!

TechnikEmpire commented 3 years ago

@ClosetBugSlayer nice, will run this. @bitcrazed

bitcrazed commented 2 years ago

Thanks for sharing the repro case @ClosetBugSlayer - re-opening while we take a look. I and a bunch of people are OOF on vacation at the moment, so there may be a delay in replying, but we are looking. Stay tuned.

bitcrazed commented 2 years ago

Hey @ClosetBugSlayer & others on this thread: Summarizing some feedback from the team:

The handle leak is due to a subtle API usage bug. In both samples Shutdown() is not being called on the media source created by the source resolver.

The documentation clearly states that it’s the application’s responsibility to shutdown the media source and not doing so can lead to resource leaks:

“If the application creates the media source, either directly or through the source resolver, the application is responsible for calling Shutdown to avoid memory or resource leaks.”

Note: The sample above, uses WIL's unique_call<...> on line 5 which ensures MFShutdown() is called when it falls out of scope.

  unique_call<decltype(&::MFShutdown), ::MFShutdown> mfshutdown;

Hope this helps resolve the issues reported.

AdamBraden commented 1 day ago

Closing issue as per last comment