Skip to content

Commit 697164a

Browse files
committed
Fixing bug in http_listener that can cause hangs on single core machines on Windows.
1 parent 9fcc601 commit 697164a

File tree

2 files changed

+50
-3
lines changed

2 files changed

+50
-3
lines changed

Release/src/http/listener/http_server_httpsys.cpp

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,17 @@ void http_windows_server::receive_requests()
425425
{
426426
HTTP_REQUEST p_request;
427427
ULONG bytes_received;
428-
for(;;)
428+
429+
// Oversubscribe since this is a blocking call and we don't want to count
430+
// towards the concurrency runtime's thread count. A more proper fix
431+
// would be to use Overlapped I/O and asynchronously call HttpReceiveHttpRequest.
432+
// This requires additional work to be careful sychronizing with the listener
433+
// shutdown. This is much easier especially given the http_listener is 'experimental'
434+
// and with VS2015 PPL tasks run on the threadpool.
435+
#if _MSC_VER < 1900
436+
concurrency::Context::Oversubscribe(true);
437+
#endif
438+
for (;;)
429439
{
430440
unsigned long error_code = HttpReceiveHttpRequest(
431441
m_hRequestQueue,
@@ -436,7 +446,7 @@ void http_windows_server::receive_requests()
436446
&bytes_received,
437447
0);
438448

439-
if(error_code != NO_ERROR && error_code != ERROR_MORE_DATA)
449+
if (error_code != NO_ERROR && error_code != ERROR_MORE_DATA)
440450
{
441451
break;
442452
}
@@ -447,7 +457,9 @@ void http_windows_server::receive_requests()
447457
http_request msg = http_request::_create_request(std::move(pRequestContext));
448458
pContext->async_process_request(p_request.RequestId, msg, bytes_received);
449459
}
450-
}
460+
#if _MSC_VER < 1900
461+
concurrency::Context::Oversubscribe(false);
462+
#endif}
451463

452464
pplx::task<void> http_windows_server::respond(http::http_response response)
453465
{

Release/tests/functional/http/listener/connections_and_errors.cpp

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,41 @@ TEST_FIXTURE(uri_address, save_request_reply)
143143
listener.close().wait();
144144
}
145145

146+
TEST_FIXTURE(uri_address, single_core_request)
147+
{
148+
#ifdef _MSC_VER
149+
// Fake having a scheduler with only 1 core.
150+
concurrency::CurrentScheduler::Create(concurrency::SchedulerPolicy(2, 1, Concurrency::MinConcurrency, 1, Concurrency::MaxConcurrency));
151+
#endif
152+
153+
http_listener listener(m_uri);
154+
listener.open().wait();
155+
test_http_client::scoped_client client(m_uri);
156+
test_http_client * p_client = client.client();
157+
158+
listener.support([](http_request request)
159+
{
160+
request.reply(status_codes::OK).get();
161+
});
162+
VERIFY_ARE_EQUAL(0, p_client->request(methods::POST, U("")));
163+
164+
// Don't wait on the task otherwise it could inline allowing other tasks to run on the scheduler.
165+
std::atomic_flag responseEvent = ATOMIC_FLAG_INIT;
166+
responseEvent.test_and_set();
167+
p_client->next_response().then([&](test_response *p_response)
168+
{
169+
http_asserts::assert_test_response_equals(p_response, status_codes::OK);
170+
responseEvent.clear();
171+
});
172+
while (responseEvent.test_and_set()) {}
173+
174+
listener.close().wait();
175+
176+
#ifdef _MSC_VER
177+
concurrency::CurrentScheduler::Detach();
178+
#endif
179+
}
180+
146181
TEST_FIXTURE(uri_address, save_request_response)
147182
{
148183
http_listener listener(m_uri);

0 commit comments

Comments
 (0)