WinInet and InternetOpen

Question

The documentation states that InternetOpen can be called multiple times without any issues. My question though is should I be calling InternetCloseHandle on handle returned by it multiple times?

For example, I have a class I call CAPIRequestContext, which has a handle which is returned by InternetOpen. Each one of my requests has it's own copy. Right now, I call InternetCloseHandle in the destructor, so it gets called multiple times.

I'm wondering if the following could cause issues: Thread A creates a CAPIRequestObject which calls InternetOpen and stores the handle. Thread B does the same, but then goes out of scope before Thread A exits, so Thread B calls the destructor in it's own CAPIRequestObject, which calls InternetCloseHandle on the handle returned by InternetOpen.

Should I remove the call to InternetCloseHandle in the destructors of my class? At least for the InternetHandle? I assume I should call InternetCloseHandle for the handle returned by HttpOpenRequest.

I have similar questions regarding the handle returned by InternetConnect. Are these handles shared?

Here is some sample code, minus some external code that I don't think is related to the issue:

class CAPIClient;
class CAPIRequest
{
public:
     CAPIRequestContext()
     {
         m_hConn = NULL;
         m_hInternet = NULL;
         m_hRequest = NULL;
     }

     ~CAPIRequestContext()
     {
         if (m_hRequest) InternetCloseHandle(m_hRequest);
         if (m_hConn) InternetCloseHandle(m_hConn);
         if (m_hInternet) InternetCloseHandle(m_hInternet);
     }

     bool Init(const CAPIClient &client, const std::string &uri, const std::string &method)
     {
          ATLASSERT(!(m_hInternet || m_hConn || m_hRequest));
          if (m_hInternet || m_hConn || m_hRequest) throw std::exception("Cannot init request more than once.");

          bool success = false;
          m_AuthToken = *client.m_pAuthToken;
          URI = uri;
          m_hInternet = InternetOpen("MyApp", INTERNET_OPEN_TYPE_PRECONFIG, NULL, NULL, 0);
DWORD requestTimeout = 60 * 1000; // Set timeout to 60 seconds instead of 30

          if (m_hInternet)
          {  
                InternetSetOption(m_hInternet, INTERNET_OPTION_RECEIVE_TIMEOUT, &requestTimeout, sizeof(requestTimeout));
                m_hConn = InternetConnect(m_hInternet, (LPSTR)client.m_host.c_str(), client.m_port, NULL, NULL, INTERNET_SERVICE_HTTP, 0, (DWORD_PTR)this);
                if (m_hConn) {
                      m_hRequest = HttpOpenRequest(m_hConn, method.c_str(), uri.c_str(), "HTTP/1.1", NULL, NULL, client.GetOpenRequestFlags(), 0);
                }
                if (m_hRequest)
                {
                    success = true;
                }
          }
       return success;
     }      
}

    // There are additional calls like 
    // SendRequest
    // GetData
    // GetFullResponse

private:
     // Added these methods to make sure I'm not copying the handles
enter code here
     CAPIRequestContext(const CAPIRequestContext &other) = delete;
     CAPIRequestContext& operator=(const CAPIRequestContext& other) = delete;

private:
      HINTERNET m_hInternet;
      HINTERNET m_hConn;
      HINTERNET m_hRequest;

}

Answer 1

The documentation states that InternetOpen can be called multiple times without any issues. My question though is should I be calling InternetCloseHandle on handle returned by it multiple times?

Yes, as is stated in InternetOpen() documentation:

After the calling application has finished using the HINTERNET handle returned by InternetOpen, it must be closed using the InternetCloseHandle function.

For example, I have a class I call CAPIRequestContext, which has a handle which is returned by InternetOpen. Each one of my requests has it's own copy. Right now, I call InternetCloseHandle in the destructor, so it gets called multiple times.

That would be a correct implementation, if each instance of the class calls InternetOpen(), or otherwise obtains ownership of a unique HINTERNET.

HOWEVER, do be aware that such a class needs to implement the Rule of Three. Basically, if you have to provide a destructor to release a resource, you also need to provide a copy-constructor and copy-assignment operator as well.

But, you can't call InternetCloseHandle() multiple times on the same HINTERNET, so you can't have multiple CAPIRequestContext using the same HINTERNET and all of them calling InternetCloseHandle()¹. So, your copy constructor and copy-assignment operator must either:

1. take ownership of the HINTERNET from the source CAPIRequestContext that is being copied.

2. be disabled completely to prevent copying one CAPIRequestContext to another.

In your case, I would opt for #2.

¹: You would need a per-instance flag indicating which instance can call it and which ones cannot. But this is not good class design. If you need to share an HINTERNET, you should implement reference counting semantics instead, such as provided by std::shared_ptr.

I'm wondering if the following could cause issues: Thread A creates a CAPIRequestObject which calls InternetOpen and stores the handle. Thread B does the same, but then goes out of scope before Thread A exits, so Thread B calls the destructor in it's own CAPIRequestObject, which calls InternetCloseHandle on the handle returned by InternetOpen.

That is perfectly safe, provided each CAPIRequestObject has its own HINTERNET.

Should I remove the call to InternetCloseHandle in the destructors of my class?

No, if each class instance holds a unique HINTERNET.

I assume I should call InternetCloseHandle for the handle returned by HttpOpenRequest.

Yes, as is stated in the HttpOpenRequest() documentation:

After the calling application has finished using the HINTERNET handle returned by HttpOpenRequest, it must be closed using the InternetCloseHandle function.

I have similar questions regarding the handle returned by InternetConnect. Are these handles shared?

Each HINTERNET must be closed individually.