Skip to content

Commit 0d5bfc5

Browse files
author
lexeyo
committed
fix core/server/http: fix possible HTTP server crash on shutdown in debug builds
The fix makes sure that `request_task`will not continue execution when the main task was interrupted and will not call `SetData` when `SetSendFailed`has been called already. commit_hash:f8adf6c42076a5dcdde78d4071169d6f17356840
1 parent e7703ff commit 0d5bfc5

6 files changed

Lines changed: 36 additions & 1 deletion

File tree

.mapping.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -808,6 +808,8 @@
808808
"core/functional_tests/graceful_shutdown/delaying_handler.hpp":"taxi/uservices/userver/core/functional_tests/graceful_shutdown/delaying_handler.hpp",
809809
"core/functional_tests/graceful_shutdown/main.cpp":"taxi/uservices/userver/core/functional_tests/graceful_shutdown/main.cpp",
810810
"core/functional_tests/graceful_shutdown/static_config.yaml":"taxi/uservices/userver/core/functional_tests/graceful_shutdown/static_config.yaml",
811+
"core/functional_tests/graceful_shutdown/tests-correct-shutdown/conftest.py":"taxi/uservices/userver/core/functional_tests/graceful_shutdown/tests-correct-shutdown/conftest.py",
812+
"core/functional_tests/graceful_shutdown/tests-correct-shutdown/test_correct_shutdown.py":"taxi/uservices/userver/core/functional_tests/graceful_shutdown/tests-correct-shutdown/test_correct_shutdown.py",
811813
"core/functional_tests/graceful_shutdown/tests/conftest.py":"taxi/uservices/userver/core/functional_tests/graceful_shutdown/tests/conftest.py",
812814
"core/functional_tests/graceful_shutdown/tests/test_graceful_shutdown.py":"taxi/uservices/userver/core/functional_tests/graceful_shutdown/tests/test_graceful_shutdown.py",
813815
"core/functional_tests/http2server/CMakeLists.txt":"taxi/uservices/userver/core/functional_tests/http2server/CMakeLists.txt",

core/functional_tests/graceful_shutdown/CMakeLists.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,5 @@ add_executable(${PROJECT_NAME} ${SOURCES} "main.cpp")
55
target_include_directories(${PROJECT_NAME} PRIVATE "${CMAKE_CURRENT_SOURCE_DIR}/src")
66
target_link_libraries(${PROJECT_NAME} userver::core)
77

8-
userver_chaos_testsuite_add()
8+
userver_chaos_testsuite_add(TESTS_DIRECTORY tests)
9+
userver_chaos_testsuite_add(TESTS_DIRECTORY tests-correct-shutdown)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
pytest_plugins = ['pytest_userver.plugins.core']
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import asyncio
2+
from signal import SIGTERM
3+
4+
import aiohttp
5+
import pytest
6+
import pytest_userver.utils.sync as sync
7+
8+
9+
@pytest.mark.uservice_oneshot
10+
async def test_correct_shutdown_with_long_running_request(
11+
service_daemon_instance,
12+
service_client,
13+
):
14+
# This test just checks that the server will not crash on shutdown with active long-running request
15+
long_running = asyncio.create_task(service_client.get('/test?delay=10s'))
16+
17+
service_daemon_instance.process.send_signal(SIGTERM)
18+
19+
async def server_is_down():
20+
try:
21+
await service_client.get('/ping')
22+
return False
23+
except aiohttp.ClientError:
24+
return True
25+
26+
await sync.wait(server_is_down)
27+
28+
with pytest.raises(aiohttp.ClientError):
29+
await long_running

core/src/server/net/connection_base.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@ engine::TaskWithResult<void> ConnectionBase::HandleQueueItem(const std::shared_p
191191
}
192192
} catch (const engine::WaitInterruptedException&) {
193193
LOG_DEBUG() << "Request processing interrupted";
194+
request_task.SyncCancel();
194195
is_response_chain_valid_ = false;
195196
} catch (const std::exception& e) {
196197
LOG_WARNING() << "Request failed with unhandled exception: " << e;

core/src/server/request/response_base.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ ResponseBase::~ResponseBase() noexcept {
7878
}
7979

8080
void ResponseBase::SetData(std::string data) {
81+
UASSERT(!is_sent_);
8182
create_time_ = std::chrono::steady_clock::now();
8283
data_ = std::move(data);
8384
guard_.emplace(accounter_, create_time_, data_.size());

0 commit comments

Comments
 (0)