Improved error reporting, tweaked request buffer size and added explicit cleanup of http API resources

1 files changed, 119 insertions, 16 deletions

diff --git a/zenhttp/httpsys.cpp b/zenhttp/httpsys.cpp
index fd93aa68f..00ffc51e9 100644
--- a/zenhttp/httpsys.cpp
+++ b/zenhttp/httpsys.cpp
@@ -9,6 +9,7 @@
 #if ZEN_WITH_HTTPSYS
 
 #	include <conio.h>
+#	include <mstcpip.h>
 #	pragma comment(lib, "httpapi.lib")
 
 std::wstring
@@ -339,7 +340,7 @@ struct InitialRequestHandler : public HttpSysRequestHandler
 	uint64_t m_CurrentPayloadOffset = 0;
 	uint64_t m_ContentLength		= ~uint64_t(0);
 	IoBuffer m_PayloadBuffer;
-	UCHAR	 m_RequestBuffer[512 + sizeof(HTTP_REQUEST)];
+	UCHAR	 m_RequestBuffer[4096 + sizeof(HTTP_REQUEST)];
 };
 
 /**
@@ -744,6 +745,8 @@ HttpSysServer::~HttpSysServer()
 {
 	if (m_IsHttpInitialized)
 	{
+		Cleanup();
+
 		HttpTerminate(HTTP_INITIALIZE_SERVER, nullptr);
 	}
 }
@@ -751,13 +754,13 @@ HttpSysServer::~HttpSysServer()
 void
 HttpSysServer::Initialize(const wchar_t* UrlPath)
 {
-	// check(bIsOk);
-
 	ULONG Result = HttpCreateServerSession(HTTPAPI_VERSION_2, &m_HttpSessionId, 0);
 
 	if (Result != NO_ERROR)
 	{
-		// Flag error
+		spdlog::error("Failed to create server session for '{}': {x}", WideToUtf8(UrlPath), Result);
+
+		m_IsOk = false;
 
 		return;
 	}
@@ -766,29 +769,33 @@ HttpSysServer::Initialize(const wchar_t* UrlPath)
 
 	if (Result != NO_ERROR)
 	{
-		// Flag error
+		spdlog::error("Failed to create URL group for '{}': {x}", WideToUtf8(UrlPath), Result);
 
 		return;
 	}
 
 	m_BaseUri = UrlPath;
 
-	Result = HttpAddUrlToUrlGroup(m_HttpUrlGroupId, UrlPath, /* #TODO UrlContext */ HTTP_URL_CONTEXT(0), 0);
+	Result = HttpAddUrlToUrlGroup(m_HttpUrlGroupId, UrlPath, HTTP_URL_CONTEXT(0), 0);
 
 	if (Result != NO_ERROR)
 	{
-		// Flag error
+		spdlog::error("Failed to add base URL to URL group for '{}': {x}", WideToUtf8(UrlPath), Result);
 
 		return;
 	}
 
 	HTTP_BINDING_INFO HttpBindingInfo = {{0}, 0};
 
-	Result = HttpCreateRequestQueue(HTTPAPI_VERSION_2, NULL, NULL, 0, &m_RequestQueueHandle);
+	Result = HttpCreateRequestQueue(HTTPAPI_VERSION_2,
+									/* Name */ nullptr,
+									/* SecurityAttributes */ nullptr,
+									/* Flags */ 0,
+									&m_RequestQueueHandle);
 
 	if (Result != NO_ERROR)
 	{
-		// Flag error!
+		spdlog::error("Failed to create request queue for '{}': {x}", WideToUtf8(UrlPath), Result);
 
 		return;
 	}
@@ -800,16 +807,43 @@ HttpSysServer::Initialize(const wchar_t* UrlPath)
 
 	if (Result != NO_ERROR)
 	{
-		// Flag error!
+		spdlog::error("Failed to set server binding property for '{}': {x}", WideToUtf8(UrlPath), Result);
 
 		return;
 	}
 
 	// Create I/O completion port
 
-	m_ThreadPool.CreateIocp(m_RequestQueueHandle, HttpSysTransaction::IoCompletionCallback, this);
+	m_ThreadPool.CreateIocp(m_RequestQueueHandle, HttpSysTransaction::IoCompletionCallback, /* Context */ this);
 
-	// Check result!
+	if (!m_ThreadPool.Iocp())
+	{
+		spdlog::error("Failed to create IOCP for '{}': {x}", WideToUtf8(UrlPath), Result);
+	}
+}
+
+void
+HttpSysServer::Cleanup()
+{
+	++m_IsShuttingDown;
+
+	if (m_RequestQueueHandle)
+	{
+		HttpCloseRequestQueue(m_RequestQueueHandle);
+		m_RequestQueueHandle = nullptr;
+	}
+
+	if (m_HttpUrlGroupId)
+	{
+		HttpCloseUrlGroup(m_HttpUrlGroupId);
+		m_HttpUrlGroupId = 0;
+	}
+
+	if (m_HttpSessionId)
+	{
+		HttpCloseServerSession(m_HttpSessionId);
+		m_HttpSessionId = 0;
+	}
 }
 
 void
@@ -870,6 +904,11 @@ HttpSysServer::OnHandlingRequest()
 void
 HttpSysServer::IssueNewRequestMaybe()
 {
+	if (m_IsShuttingDown)
+	{
+		return;
+	}
+
 	if (m_PendingRequests.load(std::memory_order::relaxed) >= m_MaxPendingRequests)
 	{
 		return;
@@ -1173,7 +1212,8 @@ InitialRequestHandler::IssueRequest()
 	}
 	else
 	{
-		static const uint64_t kMaxBytesPerApiCall = 64 * 1024;
+		// The http.sys team recommends limiting the size to 128KB
+		static const uint64_t kMaxBytesPerApiCall = 128 * 1024;
 
 		uint64_t	   BytesToRead		   = m_ContentLength - m_CurrentPayloadOffset;
 		const uint64_t BytesToReadThisCall = zen::Min(BytesToRead, kMaxBytesPerApiCall);
@@ -1208,17 +1248,80 @@ InitialRequestHandler::IssueRequest()
 HttpSysRequestHandler*
 InitialRequestHandler::HandleCompletion(ULONG IoResult, ULONG_PTR NumberOfBytesTransferred)
 {
-	ZEN_UNUSED(IoResult);
-	ZEN_UNUSED(NumberOfBytesTransferred);
-
 	auto _ = MakeGuard([&] { m_IsInitialRequest = false; });
 
+	switch (IoResult)
+	{
+		case ERROR_OPERATION_ABORTED:
+			return nullptr;
+
+		case ERROR_MORE_DATA:
+			// Insufficient buffer space
+			break;
+	}
+
 	// Route requests
 
 	try
 	{
 		HTTP_REQUEST* HttpReq = HttpRequest();
 
+#	if 0
+		for (int i = 0; i < HttpReq->RequestInfoCount; ++i)
+		{
+			auto& ReqInfo = HttpReq->pRequestInfo[i];
+
+			switch (ReqInfo.InfoType)
+			{
+				case HttpRequestInfoTypeRequestTiming:
+					{
+						const HTTP_REQUEST_TIMING_INFO* TimingInfo = reinterpret_cast<HTTP_REQUEST_TIMING_INFO*>(ReqInfo.pInfo);
+
+						spdlog::info("");
+					}
+					break;
+				case HttpRequestInfoTypeAuth:
+					spdlog::info("");
+					break;
+				case HttpRequestInfoTypeChannelBind:
+					spdlog::info("");
+					break;
+				case HttpRequestInfoTypeSslProtocol:
+					spdlog::info("");
+					break;
+				case HttpRequestInfoTypeSslTokenBindingDraft:
+					spdlog::info("");
+					break;
+				case HttpRequestInfoTypeSslTokenBinding:
+					spdlog::info("");
+					break;
+				case HttpRequestInfoTypeTcpInfoV0:
+					{
+						const TCP_INFO_v0* TcpInfo = reinterpret_cast<const TCP_INFO_v0*>(ReqInfo.pInfo);
+
+						spdlog::info("");
+					}
+					break;
+				case HttpRequestInfoTypeRequestSizing:
+					{
+						const HTTP_REQUEST_SIZING_INFO* SizingInfo = reinterpret_cast<const HTTP_REQUEST_SIZING_INFO*>(ReqInfo.pInfo);
+						spdlog::info("");
+					}
+					break;
+				case HttpRequestInfoTypeQuicStats:
+					spdlog::info("");
+					break;
+				case HttpRequestInfoTypeTcpInfoV1:
+					{
+						const TCP_INFO_v1* TcpInfo = reinterpret_cast<const TCP_INFO_v1*>(ReqInfo.pInfo);
+
+						spdlog::info("");
+					}
+					break;
+			}
+		}
+#	endif
+
 		if (HttpService* Service = reinterpret_cast<HttpService*>(HttpReq->UrlContext))
 		{
 			if (m_IsInitialRequest)