Skip to content

Commit bf8fe49

Browse files
authored
fix: ensure Python backend shm cleanup on forced shutdown (#450)
* fix: ensure Python backend shm cleanup on forced shutdown Register parent-owned shm regions for atexit cleanup, remove regions explicitly in TerminateStub, and honor stub-timeout-seconds during stub teardown to avoid orphaned regions when server exit times out. * style: fix pre-commit (copyright bump + clang-format) - Bump copyright to 2021-2026 on src/shm_manager.{cc,h}. - Collapse wrapped stub_timeout_seconds_ assignment per clang-format. * fix: address Greptile review — bounded total shutdown budget - Enforce stub_timeout_seconds_ as a single total budget across the finalize-wait and process-exit-wait phases; healthy teardown previously could take up to 2 * stub_timeout_seconds_. - Clamp the Pop timeout to INT_MAX to avoid narrowing when passing int64_t into MessageQueue::Pop(int const&). - Re-poll waitpid once after the final sleep window in WaitForStubProcessWithTimeout so a stub that exits during that second is not force-killed unnecessarily. * polish: log atexit registration failure; trim comments - Log a warning to stderr if std::atexit registration fails so the loss of the last-resort shm cleanup is visible; primary cleanup path is unaffected. - Tighten inline comments to explain only intent/constraints, not mechanics. * refactor: inline shm cleanup callback into atexit registration CleanupParentShmRegions was only referenced as the std::atexit callback inside RegisterParentShmRegion. Inline it as a capture-less lambda so the teardown logic lives next to the registration and there's one fewer top-level helper in the anonymous namespace. No behavior change.
1 parent f9088ff commit bf8fe49

4 files changed

Lines changed: 130 additions & 7 deletions

File tree

src/shm_manager.cc

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2021-2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
1+
// Copyright 2021-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
22
//
33
// Redistribution and use in source and binary forms, with or without
44
// modification, are permitted provided that the following conditions
@@ -26,13 +26,54 @@
2626

2727
#include "shm_manager.h"
2828

29+
#include <atomic>
2930
#include <boost/interprocess/managed_external_buffer.hpp>
3031
#include <boost/interprocess/mapped_region.hpp>
3132
#include <boost/interprocess/shared_memory_object.hpp>
33+
#include <cstdlib>
3234
#include <iostream>
35+
#include <mutex>
36+
#include <unordered_set>
3337

3438
namespace triton { namespace backend { namespace python {
3539

40+
namespace {
41+
42+
std::mutex parent_shm_regions_mu;
43+
std::unordered_set<std::string> parent_shm_regions;
44+
std::atomic<bool> parent_shm_atexit_registered{false};
45+
46+
void
47+
RegisterParentShmRegion(const std::string& shm_region_name)
48+
{
49+
{
50+
std::lock_guard<std::mutex> lock(parent_shm_regions_mu);
51+
parent_shm_regions.insert(shm_region_name);
52+
}
53+
if (!parent_shm_atexit_registered.exchange(true)) {
54+
if (std::atexit([]() {
55+
std::lock_guard<std::mutex> lock(parent_shm_regions_mu);
56+
for (const auto& region : parent_shm_regions) {
57+
bi::shared_memory_object::remove(region.c_str());
58+
}
59+
parent_shm_regions.clear();
60+
}) != 0) {
61+
std::cerr << "python_backend: failed to register atexit shm cleanup "
62+
"handler; relying on TerminateStub for cleanup"
63+
<< std::endl;
64+
}
65+
}
66+
}
67+
68+
void
69+
UnregisterParentShmRegion(const std::string& shm_region_name)
70+
{
71+
std::lock_guard<std::mutex> lock(parent_shm_regions_mu);
72+
parent_shm_regions.erase(shm_region_name);
73+
}
74+
75+
} // namespace
76+
3677
void
3778
CUDAMemoryPoolManager::SetCUDAPoolAddress(
3879
const int32_t device_id, void* cuda_pool_address)
@@ -139,6 +180,7 @@ SharedMemoryManager::SharedMemoryManager(
139180
if (create) {
140181
*total_size_ = current_capacity_;
141182
new (shm_mutex_) bi::interprocess_mutex;
183+
RegisterParentShmRegion(shm_region_name_);
142184
}
143185
}
144186

@@ -226,9 +268,17 @@ SharedMemoryManager::FreeMemory()
226268

227269

228270
SharedMemoryManager::~SharedMemoryManager() noexcept(false)
271+
{
272+
RemoveShmRegion();
273+
}
274+
275+
void
276+
SharedMemoryManager::RemoveShmRegion()
229277
{
230278
if (delete_region_) {
231279
bi::shared_memory_object::remove(shm_region_name_.c_str());
280+
UnregisterParentShmRegion(shm_region_name_);
281+
delete_region_ = false;
232282
}
233283
}
234284

src/shm_manager.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2021-2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
1+
// Copyright 2021-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
22
//
33
// Redistribution and use in source and binary forms, with or without
44
// modification, are permitted provided that the following conditions
@@ -194,6 +194,9 @@ class SharedMemoryManager {
194194

195195
void SetDeleteRegion(bool delete_region);
196196

197+
// Idempotent: removes the parent-owned shm region and clears delete_region_.
198+
void RemoveShmRegion();
199+
197200
std::unique_ptr<CUDAMemoryPoolManager>& GetCUDAMemoryPoolManager()
198201
{
199202
return cuda_memory_pool_manager_;

src/stub_launcher.cc

Lines changed: 71 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@
2626

2727
#include "stub_launcher.h"
2828

29+
#include <algorithm>
30+
#include <chrono>
2931
#include <filesystem>
32+
#include <limits>
3033

3134
#include "pb_utils.h"
3235
#include "python_be.h"
@@ -42,7 +45,7 @@ namespace triton { namespace backend { namespace python {
4245
StubLauncher::StubLauncher(const std::string stub_process_kind)
4346
: parent_pid_(0), is_initialized_(false),
4447
stub_process_kind_(stub_process_kind), model_instance_name_(""),
45-
device_id_(0), kind_("")
48+
device_id_(0), kind_(""), stub_timeout_seconds_(30)
4649
{
4750
}
4851

@@ -51,7 +54,7 @@ StubLauncher::StubLauncher(
5154
const int32_t device_id, const std::string kind)
5255
: is_initialized_(false), stub_process_kind_(stub_process_kind),
5356
model_instance_name_(model_instance_name), device_id_(device_id),
54-
kind_(kind)
57+
kind_(kind), stub_timeout_seconds_(30)
5558
{
5659
}
5760

@@ -64,6 +67,7 @@ StubLauncher::Initialize(ModelState* model_state)
6467
shm_growth_byte_size_ = model_state->StateForBackend()->shm_growth_byte_size;
6568
shm_message_queue_size_ =
6669
model_state->StateForBackend()->shm_message_queue_size;
70+
stub_timeout_seconds_ = model_state->StateForBackend()->stub_timeout_seconds;
6771
python_execution_env_ = model_state->PythonExecutionEnv();
6872
python_lib_ = model_state->StateForBackend()->python_lib;
6973
model_state->ModelConfig().Write(&model_config_buffer_);
@@ -801,15 +805,33 @@ void
801805
StubLauncher::TerminateStub()
802806
{
803807
if (is_initialized_) {
808+
// Single teardown budget: finalize + wait share stub_timeout_seconds_.
804809
bool force_kill = false;
810+
int64_t remaining_seconds = stub_timeout_seconds_;
805811
if (is_healthy_) {
812+
const int64_t total_timeout_ms = stub_timeout_seconds_ * 1000;
813+
// Clamp to INT_MAX; MessageQueue::Pop takes int.
814+
const int pop_timeout_ms = static_cast<int>(std::min<int64_t>(
815+
total_timeout_ms,
816+
static_cast<int64_t>(std::numeric_limits<int>::max())));
806817
// Finalize command does not have any arguments.
807818
std::unique_ptr<IPCMessage> ipc_message =
808819
IPCMessage::Create(shm_pool_, false /* inline_response */);
809820

810821
ipc_message->Command() = PYTHONSTUB_FinalizeRequest;
811822
stub_message_queue_->Push(ipc_message->ShmHandle());
812-
parent_message_queue_->Pop();
823+
bool success = false;
824+
const auto pop_start = std::chrono::steady_clock::now();
825+
parent_message_queue_->Pop(pop_timeout_ms, success);
826+
const int64_t pop_elapsed_s =
827+
std::chrono::duration_cast<std::chrono::seconds>(
828+
std::chrono::steady_clock::now() - pop_start)
829+
.count();
830+
remaining_seconds =
831+
std::max<int64_t>(0, stub_timeout_seconds_ - pop_elapsed_s);
832+
if (!success) {
833+
force_kill = true;
834+
}
813835

814836
stub_message_queue_.reset();
815837
parent_message_queue_.reset();
@@ -820,11 +842,15 @@ StubLauncher::TerminateStub()
820842

821843
if (force_kill) {
822844
KillStubProcess();
823-
} else {
824-
WaitForStubProcess();
845+
} else if (!WaitForStubProcessWithTimeout(remaining_seconds)) {
846+
KillStubProcess();
825847
}
826848
}
827849

850+
if (shm_pool_ != nullptr) {
851+
shm_pool_->RemoveShmRegion();
852+
}
853+
828854
// First destroy the IPCControl. This makes sure that IPCControl is
829855
// destroyed before the shared memory manager goes out of scope.
830856
ipc_control_.reset();
@@ -924,10 +950,50 @@ StubLauncher::WaitForStubProcess()
924950
// Added this check to ensure server doesn't hang waiting after stub
925951
// process has already be killed and cannot be waited on
926952
waitpid(stub_pid_, &status, 0);
953+
stub_pid_ = 0;
927954
}
928955
#endif
929956
}
930957

958+
bool
959+
StubLauncher::WaitForStubProcessWithTimeout(int64_t timeout_seconds)
960+
{
961+
#ifdef _WIN32
962+
WaitForStubProcess();
963+
return true;
964+
#else
965+
if (stub_pid_ == 0) {
966+
return true;
967+
}
968+
969+
for (int64_t elapsed = 0; elapsed < timeout_seconds; ++elapsed) {
970+
int status;
971+
pid_t ret = waitpid(stub_pid_, &status, WNOHANG);
972+
if (ret == stub_pid_) {
973+
stub_pid_ = 0;
974+
return true;
975+
}
976+
if (ret == -1) {
977+
stub_pid_ = 0;
978+
return true;
979+
}
980+
sleep(1);
981+
}
982+
983+
// Stub may have exited during the last sleep(1); recheck before killing.
984+
{
985+
int status;
986+
pid_t ret = waitpid(stub_pid_, &status, WNOHANG);
987+
if (ret == stub_pid_ || ret == -1) {
988+
stub_pid_ = 0;
989+
return true;
990+
}
991+
}
992+
993+
return false;
994+
#endif
995+
}
996+
931997
#ifdef TRITON_ENABLE_GPU
932998
void
933999
StubLauncher::ShareCUDAMemoryPool(

src/stub_launcher.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,9 @@ class StubLauncher {
161161
// Wait for stub process
162162
void WaitForStubProcess();
163163

164+
// Wait for stub process with timeout. Returns true if the stub exited.
165+
bool WaitForStubProcessWithTimeout(int64_t timeout_seconds);
166+
164167
#ifndef _WIN32
165168
// FIXME [DLIS-5969]: Enable for Windows when custom execution environments
166169
// are supported.
@@ -199,6 +202,7 @@ class StubLauncher {
199202
int64_t shm_default_byte_size_;
200203
int64_t shm_growth_byte_size_;
201204
int64_t shm_message_queue_size_;
205+
int64_t stub_timeout_seconds_;
202206

203207
// Path to python execution environment
204208
std::string path_to_libpython_;

0 commit comments

Comments
 (0)