5

Sorry for such a long question. It's just I've spent several days trying to solve my problem, and I'm exhausted.

I'm trying to use WinINet in an asynchronous mode. And I must say... this is simply insane. I really can't understand this. It does so many things, but unfortunately its asynchronous API is so much poorly designed that it just can't be used in a serious application with high stability demands.

My problem is the following: I need to do a lot of HTTP/HTTPS transactions serially, whereas I also need to be able to abort them immediately at request.

I was going to use WinINet in the followig way:

  1. Initialize WInINet usage via InternetOpen function with INTERNET_FLAG_ASYNC flag.
  2. Install a global callback function (via InternetSetStatusCallback).

Now, in order to perform a transaction that's what I thought to do:

  1. Allocate a per-transaction structure with various members describing the transaction state.
  2. Call InternetOpenUrl to initiate the transaction. In the asynchronous mode it usually immediately returns with an error, which is ERROR_IO_PENDING. One of its parameters is the 'context', the value which will be passed to the callback function. We set it to the pointer to the per-transaction state structure.
  3. Very shortly after this the global callback function is called (from another thread) with status INTERNET_STATUS_HANDLE_CREATED. At this moment we save the WinINet session handle.
  4. Eventually callback function is invoked with INTERNET_STATUS_REQUEST_COMPLETE when the transaction is complete. This allows us to use some notification mechanism (such as setting an event) to notify the originating thread that the transaction is complete.
  5. The thread that issued the transaction realizes that it's complete. Then it does the cleanup: closes the WinINet session handle (by InternetCloseHandle), and deletes the state structure.

So far there seems to be no problem.

How to abort a transaction which is in the middle of execution? One way is to close the appropriate WinINet handle. And since WinINet doesn't have functions such as InternetAbortXXXX - closing the handle seems to be the only way to abort.

Indeed this worked. Such a transaction completes immediately with ERROR_INTERNET_OPERATION_CANCELLED error code. But here all the problems begin...

The first unpleasant surprise that I've encountered is that WinINet tends to invoke sometimes the callback function for the transaction even after it has already been aborted. According to the MSDN the INTERNET_STATUS_HANDLE_CLOSING is the last invocation of the callback function. But it's a lie. What I see is that sometimes there's a consequent INTERNET_STATUS_REQUEST_COMPLETE notification for the same handle.

I also tried to disable the callback function for the transaction handle right before closing it, but this didn't help. It seems that the callback invocation mechanism of the WinINet is asynchronous. Hence - it may call the callback function even after the transaction handle has been closed.

This imposes a problem: as long as WinINet may call the callback function - obviously I can't free the transaction state structure. But how the hell do I know whether or not WinINet will be so kind to call it? From what I saw - there's no consistency.

Nevertheless I've worked this around. Instead I now keep a global map (protected by the critical section of course) of allocated transaction structures. Then, inside the callback function I ensure that the transaction indeed exists and lock it for the duration of the callback invocation.

But then I've discovered another problem, which I couldn't solve so far. It arises when I abort a transaction very shortly after it's started.

What happens is that I call InternetOpenUrl, which returns the ERROR_IO_PENDING error code. Then I just wait (very short usually) until the callback function will be called with the INTERNET_STATUS_HANDLE_CREATED notification. Then - the transaction handle is saved, so that now we have an opportunity to abort without handle/resource leaks, and we may go ahead.

I tried to do the abort exactly after this moment. That is, close this handle immediately after I receive it. Guess what happens? WinINet crashes, invalid memory access! And this is not related to whatever I do in the callback function. The callback function isn't even called, the crash is somewhere deep inside the WinINet.

On the other hand if I wait for the next notification (such as 'resolving name') - usually it works. But sometimes crashes as well! The problem seems to disappear if I put some minimal Sleep between obtaining the handle and closing it. But obviously this can't be accepted as a serious solution.

All this makes me conclude: The WinINet is poorly designed.

  • There's no strict definition about the scope of the callback function invocation for the specific session (transaction).
  • There's no strict definition about the moment from which I'm allowed to close WinINet handle.
  • Who knows what else?

Am I wrong? Is that something that I don't understand? Or WinINet just can't be safely used?

EDIT:

This is the minimal code block that demonstrates the 2nd issue: crash. I've removed all the error handling and etc.

HINTERNET g_hINetGlobal;

struct Context
{
    HINTERNET m_hSession;
    HANDLE m_hEvent;
};

void CALLBACK INetCallback(HINTERNET hInternet, DWORD_PTR dwCtx, DWORD dwStatus, PVOID pInfo, DWORD dwInfo)
{
    if (INTERNET_STATUS_HANDLE_CREATED == dwStatus)
    {
        Context* pCtx = (Context*) dwCtx;
        ASSERT(pCtx && !pCtx->m_hSession);

        INTERNET_ASYNC_RESULT* pRes = (INTERNET_ASYNC_RESULT*) pInfo;
        ASSERT(pRes);
        pCtx->m_hSession = (HINTERNET) pRes->dwResult;

        VERIFY(SetEvent(pCtx->m_hEvent));
    }
}

void FlirtWInet()
{
    g_hINetGlobal = InternetOpen(NULL, INTERNET_OPEN_TYPE_PRECONFIG, NULL, NULL, INTERNET_FLAG_ASYNC);
    ASSERT(g_hINetGlobal);
    InternetSetStatusCallback(g_hINetGlobal, INetCallback);

    for (int i = 0; i < 100; i++)
    {
        Context ctx;
        ctx.m_hSession = NULL;
        VERIFY(ctx.m_hEvent = CreateEvent(NULL, FALSE, FALSE, NULL));

        HINTERNET hSession = InternetOpenUrl(
            g_hINetGlobal,
            _T("http://ww.google.com"),
            NULL, 0,
            INTERNET_FLAG_NO_UI | INTERNET_FLAG_PRAGMA_NOCACHE | INTERNET_FLAG_RELOAD,
            DWORD_PTR(&ctx));

        if (hSession)
            ctx.m_hSession = hSession;
        else
        {
            ASSERT(ERROR_IO_PENDING == GetLastError());
            WaitForSingleObject(ctx.m_hEvent, INFINITE);
            ASSERT(ctx.m_hSession);
        }

        VERIFY(InternetCloseHandle(ctx.m_hSession));
        VERIFY(CloseHandle(ctx.m_hEvent));

    }

    VERIFY(InternetCloseHandle(g_hINetGlobal));
}

Usually on first/second iteration the application crashes. One of the thread created by the WinINet generates an access violation:

Access violation reading location 0xfeeefeee.

Worth to note that the above address has special meaning to the code written in C++ (at least MSVC). AFAIK when you delete an object that has a vtable (i.e. - has virtual functions) - it's set to the above address. So that it's an attempt to call a virtual function of an already-deleted object.

valdo
  • 12,632
  • 2
  • 37
  • 67
  • 4
    I agree it is a bit difficult to use, but I'm not seeing the same problems. For your first issue, are you sure INTERNET_STATUS_HANDLE_CLOSING and INTERNET_STATUS_REQUEST_COMPLETE are sent for the **same** handle? I get different handles in my callback, though oddly the REQUEST_COMPLETE seems to give me the handle I get from InternetOpen. Also, your second scenario does not cause a crash for me. I am using Windows XP SP3. – Luke Aug 05 '10 at 20:07
  • Luke, thanks for the response. Yes, the INTERNET_STATUS_REQUEST_COMPLETE comes for another handle. But it comes with the same 'context' value, which I use to indentify the state of my request. Means - if this notification arrives after I delete this state - there will be a problem. But, as I mentioned, this can be worked around. The other problem seems much more cruel. And this happens frequently (though not always). I'm talking about closing the session handle **immediately** after it's reported by WinINet. Then, after a short period, WinINet code generates access violation. – valdo Aug 05 '10 at 21:46
  • 4
    I think internally InternetOpenUrl is equivalent to InternetConnect + HttpOpenRequest (for http resources), so the callback gets invoked with both the Connect handle and the Request handle (depending on what particular events occur). Since InternetOpenUrl only takes one context parameter, it gets passed for both handles. I've tried many times, but I cannot get it to crash when calling InternetCloseHandle during HANDLE_CREATED. Not sure what the problem could be. – Luke Aug 05 '10 at 23:32
  • Hm... this is interesting. I'll try to use InternetConnect + HttpOpenRequest instead of InternetOpenUrl. It's a bit restricting since now we only support http/https, but since that's what I **really** need - worth trying. Nevertheless I've edited the question. It now includes the code that demonstrates the crash – valdo Aug 06 '10 at 08:27
  • 4
    Your example code crashes for me; interesting. Seems like it is crashing while trying to modify the Connection object, which appears to have been deleted. I wonder if that object is not properly secured with a critical section or something. – Luke Aug 08 '10 at 13:37
  • Perhaps. Nevertheless I've now modified the code to do what you suggested, and there's no problem Thanks. P.S. I really wanted to vote for your post, but since it was "comment" - I couldn't do it. – valdo Aug 08 '10 at 16:56
  • I can't get the example code to crash even with 100000 iterations. I'm on Win 7 SP1 64-bit, but compiling the code for 32-bit. – James Johnston Mar 07 '16 at 18:03

2 Answers2

8

declaration of Context ctx is the source of problem, it is declared within a for(;;) loop, so it's a local variable created for each loop, it will be destroyed and no longer accessible at end of each loop.

as a result, when a callback is invoked, ctx has been destroyed already, pointer being passed to callback points to a destroyed ctx, invalid memory pointer causes the crash.

zjp
  • 81
  • 1
  • 2
2

Special thanks to Luke.

All the problems disappear when I explicitly use InternetConnect + HttpOpenRequest + HttpSendRequest instead of all-in-one InternetOpenUrl.

I don't receive any notifications on the request handle (not to confuse with the 'connection' handle). Plus no more crashes.

valdo
  • 12,632
  • 2
  • 37
  • 67
  • 2
    I strongly suggest you consider what zjp suggested. Your new code might have changed so that this problem is no longer occurs, but you *were* using a variable that went out of scope. – André Caron Mar 02 '11 at 16:42