4

I'm having a very bizarre problem with the IShellDispatch COM interface, more specifically with the FolderItemVerbs object, that drives me nuts!

Calling FolderItemVerbs::Release() followed by CoUninitialze() will result in crash. It's clearly reproducible, but only happens 1 out of 10 times.

The crash is an "0xC0000005: Access Violation" error. Running the problematic code in a loop 100% reproduces the crash sooner or later :-(

Please see the example program:

static int TestProc(const TCHAR *pcDirectoryName, const TCHAR *pcFileName)
{
    int iSuccess = 0;

    IShellDispatch *pShellDispatch = NULL;
    Folder *pFolder = NULL; FolderItem *pItem = NULL;
    FolderItemVerbs *pVerbs = NULL;

    HRESULT hr = CoCreateInstance(CLSID_Shell, NULL, CLSCTX_INPROC_SERVER, IID_IShellDispatch, (void**)&pShellDispatch);
    if(FAILED(hr) || (pShellDispatch ==  NULL))
    {
        iSuccess = -3;
        return iSuccess;
    }

    variant_t vaDirectory(pcDirectoryName);
    hr = pShellDispatch->NameSpace(vaDirectory, &pFolder);
    if(FAILED(hr) || (pFolder == NULL))
    {
        iSuccess = -4;
        pShellDispatch->Release();
        return iSuccess;
    }

    variant_t vaFileName(pcFileName);
    hr = pFolder->ParseName(vaFileName, &pItem);
    if(FAILED(hr) || (pItem == NULL))
    {
        iSuccess = -5;
        pFolder->Release();
        pShellDispatch->Release();
        return iSuccess;
    }

    hr = pItem->Verbs(&pVerbs);
    if(FAILED(hr) || (pVerbs == NULL))
    {
        iSuccess = -6;
        pItem->Release();
        pFolder->Release();
        pShellDispatch->Release();
        return iSuccess;
    }

    /* Here we would do something with the FolderItemVerbs */

    pVerbs->Release(); pVerbs = NULL; //If this line is commented out, we don't get a crash, but a massive memory leak!
    pItem->Release(); pItem = NULL;
    pFolder->Release(); pFolder = NULL;
    pShellDispatch->Release(); pShellDispatch = NULL;

    iSuccess = 1;
    return iSuccess;
}

//-----------------------------------------------------------------------------

static unsigned __stdcall ThreadProc(void* pArguments)
{
    HRESULT hr = CoInitialize(NULL);
    if((hr == S_OK) || (hr == S_FALSE))
    {
        threadParam_t *params = (threadParam_t*) pArguments;
        params->returnValue = TestProc(params->pcDirectoryName, params->pcFileName);
        CoUninitialize();
    }
    else
    {
        if(threadParam_t *params = (threadParam_t*) pArguments)
        {
            params->returnValue = -10;
        }
    }

    return EXIT_SUCCESS;
}

Please download the complete example code is here: http://pastie.org/private/0xsnajpia9lsmgnlf2afa

Please also note that I unambiguously tracked down to crash to FolderItemVerbs, because if I never create the FolderItemVerbs object, the crash is gone immediately.

Also if I never call "pVerbs->Release()" before CoUninitialize() the crash is gone too, but this will result in a massive memleak, obviously.

Another strange thing is that the crash will NOT happen, if I run the program under the Debugger! But I can run the program, wait for the crash and then let the Debugger handle the crash.

Unfortunately the Stack Trace that I get then doesn't help much: http://pastie.org/private/cuwunlun2t5dc5lembpw

I don't think I'm doing anything wrong here. I have checked the code over and over again in the last two days. So this all seems to be a bug in FolderItemVerbs!

Has anybody encountered this before and/or can confirm that this is a bug in FolderItemVerbs? Also, is there any workaround for the problem?

Thanks in advance !!!

MuldeR
  • 577
  • 1
  • 4
  • 17
  • 2
    Your code runs well for me. Call stack suggest that the problem is on worker thread, which not yours. I suppose the problem might be in some shell extension you have installed, not with the shell itself. – Roman R. Oct 22 '12 at 19:41
  • Thank you for the answer. Unfortunately, even if this is caused by a "bad" shell extension, I would have to make sure my app works fine with the shell extension. The user might have a "bad" shell extension installed too! Is there a way I can prevent third-party shell extensions from injecting themselves into my process? Or is there anything I need to do to ensure that third-party shell extensions will "shutdown" gracefully? Regards. – MuldeR Oct 22 '12 at 21:17
  • It would certainly be helpful if you manage to isolate the issue to specific module/extension that causes the problem. With this knowledge you would have more ideas on working things around. As a guess - I would try to not call `CoUninitialize` immediately. Instead, give it some time to cool down and wait dispatching messages for a second. Quite possibly, the worker thread just needs to report it's finished and then everything will shut down nicely. – Roman R. Oct 22 '12 at 21:36
  • Another easy things for a try is to run the same code in `x64` domain. If the troubling thingy is `Win32` only, the code will work fine. – Roman R. Oct 22 '12 at 21:38
  • Actually adding a Sleep() before the CoUninitialize is one of the first things I tried. It seemed to make the problem happen less often, but it's rally an ugly workaround. It's like throwing the dice again in a "race condition". And sometimes I got the crash even with Sleep(10000). So I gave up on this and was looking for the "true" solution. Actually my app needs to be Win32, but my System and thus the Shell (and thus the Shell Extensions) are x64. Can 32-Bit Shell Extensions be active on a x64 system? And how do I track down the "bad" one? Stack Trace doesn't name a suspicious module name... – MuldeR Oct 22 '12 at 21:44
  • The point is that you can compare x64 to Win32 in terms of whether your code is stable or not (I did not suggest that you should scrap Win32 and ship x64 only). `Sleep` is good, but workers are not likely to run on their own orphaned, so I suggested [waiting dispatching messages](http://stackoverflow.com/a/12236639/868014) instead. – Roman R. Oct 22 '12 at 21:49
  • 1
    Agreed, the stack trace doesn't have anything to do with this code. Code that crashes over and over again like that is usually caused by a DLL getting unloaded too early. You'd see the notification in the VS Output window. – Hans Passant Oct 22 '12 at 21:53
  • Thanks so far! I played around with Sysinternals Autoruns and indeed, disabling the "TortoiseGit" Context Menu Handler seemed to fix the issue for me! Now: Is this something that could only be fixed by fixing the Tortoise GIT code? Or is there a workaround I could implement on my side? After all, other Shell Extensions could exhibit a similar behavior... – MuldeR Oct 22 '12 at 22:24
  • Roman R., thank you so much for the hint about dispatching messages! This indeed seems to resolve the crash, even with the TortoiseGIT Shell Extension. So it seems that was exactly what I needed. Only question that arises: Why isn't there a BIG FAT warning in the CoUninitialize() documentation telling me that I might need to do that? Best Regards. – MuldeR Oct 22 '12 at 22:54

1 Answers1

0

Thanks everybody!

Here is the "corrected" code which performs explicit message dispatching:

void DispatchPendingMessages(void)
{
    const DWORD uiTimeout = GetTickCount() + 10000;
    const HANDLE hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
    unsigned int counter = 0;
    if(hEvent)
    {
        for(;;)
        {
            MSG Message;
            while(PeekMessage(&Message, NULL, WM_NULL, WM_NULL, PM_REMOVE))
            {
                TranslateMessage(&Message);
                DispatchMessage(&Message);
            }
            const DWORD nWaitResult = MsgWaitForMultipleObjects(1, &hEvent, FALSE, 250, QS_ALLINPUT | QS_ALLPOSTMESSAGE);
            if((nWaitResult == WAIT_TIMEOUT) || (nWaitResult == WAIT_FAILED) || (GetTickCount() >= uiTimeout)) break;
        }
        CloseHandle(hEvent);
    }
}

//-----------------------------------------------------------------------------

static int TestProc(const TCHAR *pcDirectoryName, const TCHAR *pcFileName)
{
    int iSuccess = 0;

    IShellDispatch *pShellDispatch = NULL;
    Folder *pFolder = NULL; FolderItem *pItem = NULL;
    FolderItemVerbs *pVerbs = NULL;

    HRESULT hr = CoCreateInstance(CLSID_Shell, NULL, CLSCTX_INPROC_SERVER, IID_IShellDispatch, (void**)&pShellDispatch);
    if(FAILED(hr) || (pShellDispatch ==  NULL))
    {
        iSuccess = -3;
        return iSuccess;
    }

    variant_t vaDirectory(pcDirectoryName);
    hr = pShellDispatch->NameSpace(vaDirectory, &pFolder);
    if(FAILED(hr) || (pFolder == NULL))
    {
        iSuccess = -4;
        pShellDispatch->Release();
        return iSuccess;
    }

    variant_t vaFileName(pcFileName);
    hr = pFolder->ParseName(vaFileName, &pItem);
    if(FAILED(hr) || (pItem == NULL))
    {
        iSuccess = -5;
        pFolder->Release();
        pShellDispatch->Release();
        return iSuccess;
    }

    hr = pItem->Verbs(&pVerbs);
    if(FAILED(hr) || (pVerbs == NULL))
    {
        iSuccess = -6;
        pItem->Release();
        pFolder->Release();
        pShellDispatch->Release();
        return iSuccess;
    }

    /* Here we would do something with the FolderItemVerbs */

    pVerbs->Release(); pVerbs = NULL;
    pItem->Release(); pItem = NULL;
    pFolder->Release(); pFolder = NULL;
    pShellDispatch->Release(); pShellDispatch = NULL;

    iSuccess = 1;
    return iSuccess;
}

//-----------------------------------------------------------------------------

static unsigned __stdcall ThreadProc(void* pArguments)
{
    HRESULT hr = CoInitialize(NULL);
    if((hr == S_OK) || (hr == S_FALSE))
    {
        threadParam_t *params = (threadParam_t*) pArguments;
        params->returnValue = TestProc(params->pcDirectoryName, params->pcFileName);
        DispatchPendingMessages(); //This is required before CoUninitialize() to avoid crash with certain Shell Extensions !!!
        CoUninitialize();
    }
    else
    {
        if(threadParam_t *params = (threadParam_t*) pArguments)
        {
            params->returnValue = -10;
        }
    }

    return EXIT_SUCCESS;
}

Couldn't reproduce the crash with that code so far :-)

MuldeR
  • 577
  • 1
  • 4
  • 17
  • Nah, you haven't found it. Pumping a message loop for an STA thread is a hard requirement but it causes deadlock when you don't, not a crash. Your nemesis is CoFreeUnusedLibraries() and a buggy shell extension that unloads while it has a pending marshaling call. GetInterfaceFromGlobal() in your stack trace started that. Debugging your code, single stepping it, can cause CoFreeUnusedLibraries() to be called. Something you probably stopped doing when you found out it wasn't your code. Shell extensions are a bug factory, particularly the open source kind. – Hans Passant Oct 23 '12 at 00:52
  • The shellex module is perhaps positive in `DllCanUnloadNow` within CoUninit call, and this causes DLL unload while worker thread is still active. The STA-compliant wait wins some time for this worker thread to finish and make the hosting DLL truely ready for unload... That is, wait does not reliably fix the bug, but it works around a set of issues related to buggy shell extensions. – Roman R. Oct 23 '12 at 06:09
  • Hans Passant, so if running a "message pump" is a "hard" requirement, how would I do this "correctly" in my background/worker thread? If the thread is supposed to be in the message loop all the time, how do I do the actual work from the message loop? I mean, as soon as I call *something* from the loop, the loop gets "blocked" and we have a problem again, don't we? And when to "safely" exit the loop? BTW: I see a lot of code that calls CoInitialize(), does some work, and calls CoUninitialize(), so there never is any message pump. Is all that code "wrong" then? Regards – MuldeR Oct 23 '12 at 11:46
  • BTW: What about using CoWaitForMultipleHandles() instead of MsgWaitForMultipleObjects()? Shouldn't that function automatically dispatch all messages that arrive while we are waiting? And what would be the right waiting flags to use? Regards – MuldeR Oct 23 '12 at 12:41