QA - why ECB becomes invalid after returning HSE_STATUS_PENDING

Question:

Hi,

we have a ISAPI extension DLL. This DLL will receive all request for their virtual directory. After processing the request, the resulted HTML or image is sent to the client.

At the moment, the DLL is singlethreaded only (using CSingleLocks to synchronize). But due to multiple request, the DLL should become able to process the request in multiple threads.

Therefore, I overwrote the HttpExtensionProc method of the ISAPI extension class (based on CHttpServer). In HttpExtensionProc I create a CHttpServerContext based on the pECB control structure and save this CHttpServerContext object in a request queue. Then, a message is posted to all threads, that a new request was placed in the queue. The first (idle) thread who receives this message, will read and remove the first item in the request queue (again synchronized by CSingleLock). Meanwhile, my HttpExtensionProc returned HSE_STATUS_PENDING. The ECB control structure should now stay valid until I call ServerSupportFunction with HSE_REQ_DONE_WITH_SESSION for this ECB. But the ECB control structure is invalid when the thread received it from the request queue - it seems that the control structure was released by IIS in spite of returnin HSE_STATUS_PENDING.

This is my HttpExtensionProc:

DWORD CISAPIExtension::HttpExtensionProc(EXTENSION_CONTROL_BLOCK *pECB)
{
    if ( strcmp(pECB->lpszMethod, "GET") != 0 )
        return CHttpServer::HttpExtensionProc(pECB);

    ISAPIENSURE(pECB != NULL);
    CHttpServerContext context(pECB);
    pECB->dwHttpStatusCode = 0;

    if ( !InitInstance(&context) )
    {
        // ...
        return HSE_STATUS_ERROR;
    }

    return HandleRequest(&context);
}

The HandleRequest method:

DWORD CISAPIExtension::HandleRequest(CHttpServerContext* pCtxt)
{
    CString strQuery;
    if ( mstrcmp(pCtxt->m_pECB->lpszMethod, "GET") == 0 )
    {
        strQuery = pCtxt->m_pECB->lpszQueryString;
        pCtxt->m_dwBytesReceived = strQuery.GetLength();
    }

    if ( !IsDataLoaded() )
    {
        pCtxt->m_dwStatusCode = 500;
        pCtxt->m_pECB->dwHttpStatusCode = 500;
        LogError(pCtxt->m_pECB->lpszQueryString, 0, _T(
            "Could not handle request, because loading data failed!"));
        Notify(pCtxt, _T("Internal server error"), _T(
            "An error occured while processing your request."
            "Check the log file for further details."));
        return HSE_STATUS_ERROR;
    }

    strQuery.MakeLower();
    DWORD dwReturn = 0;
    // for implementing session based reqest processing
    CISAPISession* pSession = NULL; 

    if ( m_threads.HandleRequest(new CISAPIRequest(pCtxt, pSession)) )
        dwReturn = HSE_STATUS_PENDING;
    else
        dwReturn = HSE_STATUS_ERROR;

    return dwReturn;
}

m_threads is the thread manager (including the request queue). The HandleRequest method of m_threads simply adds the CISAPIRequest object (this class has only one constructor and two members: a CHttpServerContext pointer and a Session pointer) to the request queue an posts a notification message to every thread. dwReturn is HSE_STATUS_PENDING in case of success (which is true), so the return value of HttpExtensionProc will be HSE_STATUS_PENDING, too.

When I step through in debugger, my HttpExtesionProc will really return HSE_STATUS_PENDING to the

DWORD WINAPI HttpExtensionProc(EXTENSION_CONTROL_BLOCK *pECB)

method in isapi.cpp. Further debugging isn't possible, as there is no code for the calling method of the HttpExtensionProc in isapi.cpp.

Does anybody has a explanation for this behaviour. Is there any workaround? Maybe I could install a newer IIS SDK, but the solution should run on IIS 5, too.

The solution is MFC based. I'm using Visual Studio 2005 Pro for development and testing, the release version will be built using VS6 SP6. The DLL should work with IIS 5 and 6. I'm testing on WinXP Pro SP2 and IIS 5.1.

Your help is greatly appreciated!

Bye,

Answer:

Actually, I think the issue is within your code. Let me try to explain what is going on and hopefully you will see what is going awry.

The following is my summary of key events from the posted code snippet:

1. Enter CISAPIExtension::HttpExtensionProc
   
2. Create CHttpServerContext on the stack which keeps a pointer to ECB
   
3. Create CISAPIRequest on the heap which keeps pointer to CHttpServerContext and CISAPISession
   
4. Return HSE_STATUS_PENDING from CISAPIExtension::HttpExtensionProc.

   At this point, stack based memory for CHttpServerContext goes out of scope and becomes invalid. The ECB is still valid, but the memory of the pointer to ECB within CHttpServerContext is invalid.
   

5. Later, Thread attempts to use the stored stack-address of CHttpServerContext to retrieve ECB. Since that address is invalid, thread may not get a pointer to an ECB and most likely fails using it.

How to fix this? I can think of a few ways to keep the memory of the pointer to ECB valid through the asynchronous operation, but basically it depends on the memory management scheme of your code:

• Create CHttpServerContext on the heap, pass it to CISAPIRequest, and let CISAPIRequest free the CHttpServerContext
  
• Pass ECB to CISAPIRequest, create CHttpServerContext in the constructor to hold ECB, and let CISAPIRequest free the CHttpServerContext
  
• Just pass ECB to CISAPIRequest, keep a pointer to it within CISAPIRequest. Remember to call HSE_REQ_DONE_WITH_SESSION on it when CISAPIRequest no longer needs it

One of the key things to remember for asynchronous processing is to maintain the validity of the memory used throughout the entire asynchronous process. In this case, the code did not maintain the validity of the memory used for CHttpServerContext, which then fails to retain valid pointer to ECB and hence asynchronous processing cannot reliably access the ECB.

This classic failure pattern frequently shows up in the fallowing way: Due to the inherent race condition of using invalid memory, one may find that this code "never failed" while the developer developed/tested with a debug build or under non-stress conditions... and only later, when one is under the gun to produce a retail build for production or stress-test the binary, does the latent error reveal itself.

Frequently, the rigors of questioning one's code or test methodology is not strong or deep enough, and combined with developer's artificial confidence in their code or testing acumen (ahem... hubris, anyone? ;-) ), it leads to claims that either their compiler cannot create bug-free retail code, or that Windows Servers are buggy, or that IIS fails under stress. Basically, anything and anyone but themselves, and Microsoft is a big target.

Personally, when it comes to software development, I check my hubris at the door and start by assuming that I am guilty until proven innocent. And then try really hard to prove my innocence. :-)

//David