Uniformize preemptions/termination checks and implement graceful SIGINT handler #745
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| /* clang-format off */ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
| /* clang-format on */ | ||
|
|
@@ -20,7 +20,7 @@ class Callback { | |
| virtual ~Callback() {} | ||
| }; | ||
|
|
||
| enum class base_solution_callback_type { GET_SOLUTION, SET_SOLUTION, CHECK_TERMINATION }; | ||
|
|
||
| class base_solution_callback_t : public Callback { | ||
| public: | ||
|
|
@@ -56,6 +56,14 @@ class set_solution_callback_t : public base_solution_callback_t { | |
| } | ||
| }; | ||
|
|
||
| class check_termination_callback_t : public base_solution_callback_t { | ||
| public: | ||
| virtual bool check_termination() = 0; | ||
| base_solution_callback_type get_type() const override | ||
| { | ||
| return base_solution_callback_type::CHECK_TERMINATION; | ||
| } | ||
| }; | ||
| } // namespace internals | ||
|
|
||
| namespace linear_programming { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,125 @@ | ||
| /* clang-format off */ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
| /* clang-format on */ | ||
|
|
||
| #pragma once | ||
|
|
||
| #include <chrono> | ||
| #include <csignal> | ||
| #include <cstdlib> | ||
| #include <deque> | ||
| #include <functional> | ||
| #include <mutex> | ||
| #include <unordered_map> | ||
|
|
||
| namespace cuopt { | ||
|
|
||
| /** | ||
| * @brief Global singleton that handles SIGINT (Ctrl-C) and invokes registered callbacks. | ||
| * | ||
| * Components that want to respond to user interrupts register a callback via | ||
| * register_callback() and unregister via unregister_callback(). | ||
| * | ||
| * Safety feature: If the user presses Ctrl-C 5 times within 5 seconds, | ||
| * the process is forcefully terminated. | ||
| */ | ||
| class user_interrupt_handler_t { | ||
| public: | ||
| static user_interrupt_handler_t& instance() | ||
| { | ||
| static user_interrupt_handler_t instance; | ||
| return instance; | ||
| } | ||
|
|
||
| bool termination_requested() const { return terminate_signal_received_.load(); } | ||
|
|
||
| /** | ||
| * @brief Register a callback to be invoked on SIGINT. | ||
| * @param callback Function to call when interrupt is received. | ||
| * @return Registration ID for later removal. | ||
| */ | ||
| size_t register_callback(std::function<void()> callback) | ||
| { | ||
| std::lock_guard<std::mutex> lock(mutex_); | ||
| size_t id = next_id_++; | ||
| callbacks_[id] = std::move(callback); | ||
| return id; | ||
| } | ||
|
|
||
| /** | ||
| * @brief Unregister a previously registered callback. | ||
| * @param id Registration ID returned by register_callback(). | ||
| */ | ||
| void unregister_callback(size_t id) | ||
| { | ||
| std::lock_guard<std::mutex> lock(mutex_); | ||
| callbacks_.erase(id); | ||
| } | ||
|
|
||
| // Non-copyable, non-movable | ||
| user_interrupt_handler_t(const user_interrupt_handler_t&) = delete; | ||
| user_interrupt_handler_t& operator=(const user_interrupt_handler_t&) = delete; | ||
| user_interrupt_handler_t(user_interrupt_handler_t&&) = delete; | ||
| user_interrupt_handler_t& operator=(user_interrupt_handler_t&&) = delete; | ||
|
|
||
| private: | ||
| static constexpr int force_quit_threshold = 5; | ||
| static constexpr int force_quit_window_seconds = 5; | ||
|
|
||
| using time_point = std::chrono::steady_clock::time_point; | ||
|
|
||
| user_interrupt_handler_t() { previous_handler_ = std::signal(SIGINT, &handle_signal); } | ||
|
|
||
| ~user_interrupt_handler_t() | ||
| { | ||
| if (previous_handler_ != SIG_ERR) { std::signal(SIGINT, previous_handler_); } | ||
| } | ||
|
|
||
| static void handle_signal(int /*sig*/) | ||
| { | ||
| auto& self = instance(); | ||
| std::lock_guard<std::mutex> lock(self.mutex_); | ||
|
|
||
| self.terminate_signal_received_ = true; | ||
|
|
||
| auto now = std::chrono::steady_clock::now(); | ||
| auto cutoff = now - std::chrono::seconds(force_quit_window_seconds); | ||
|
|
||
| // Remove timestamps older than the window | ||
| while (!self.interrupt_times_.empty() && self.interrupt_times_.front() < cutoff) { | ||
| self.interrupt_times_.pop_front(); | ||
| } | ||
| self.interrupt_times_.push_back(now); | ||
|
|
||
| // Force quit if too many interrupts in the window | ||
| if (static_cast<int>(self.interrupt_times_.size()) >= force_quit_threshold) { | ||
| fprintf(stderr, | ||
| "Force quit: %d interrupts in %d seconds.", | ||
| force_quit_threshold, | ||
| force_quit_window_seconds); | ||
| std::_Exit(128 + SIGINT); | ||
| } | ||
|
|
||
| // Invoke all registered callbacks | ||
| for (const auto& [id, callback] : self.callbacks_) { | ||
| callback(); | ||
| } | ||
|
|
||
| auto remaining = force_quit_threshold - static_cast<int>(self.interrupt_times_.size()); | ||
| fprintf(stderr, | ||
| "Interrupt received. Stopping solver... (press Ctrl-C %d more time(s) to force quit)", | ||
| remaining); | ||
| } | ||
|
Comment on lines
+81
to
+115
There was a problem hiding this comment. Calling mutex and invoking callbacks in a signal handler is undefined behavior. Signal handlers have strict requirements on what functions can be called safely. Per POSIX, only async-signal-safe functions are permitted in signal handlers. This can cause deadlocks if SIGINT arrives while Consider one of these safer approaches:
🔎 Suggested safer pattern static void handle_signal(int /*sig*/)
{
auto& self = instance();
- std::lock_guard<std::mutex> lock(self.mutex_);
-
- self.terminate_signal_received_ = true;
-
- auto now = std::chrono::steady_clock::now();
- auto cutoff = now - std::chrono::seconds(force_quit_window_seconds);
-
- // Remove timestamps older than the window
- while (!self.interrupt_times_.empty() && self.interrupt_times_.front() < cutoff) {
- self.interrupt_times_.pop_front();
- }
- self.interrupt_times_.push_back(now);
-
- // Force quit if too many interrupts in the window
- if (static_cast<int>(self.interrupt_times_.size()) >= force_quit_threshold) {
- fprintf(stderr,
- "Force quit: %d interrupts in %d seconds.",
- force_quit_threshold,
- force_quit_window_seconds);
- std::_Exit(128 + SIGINT);
- }
-
- // Invoke all registered callbacks
- for (const auto& [id, callback] : self.callbacks_) {
- callback();
- }
-
- auto remaining = force_quit_threshold - static_cast<int>(self.interrupt_times_.size());
- fprintf(stderr,
- "Interrupt received. Stopping solver... (press Ctrl-C %d more time(s) to force quit)",
- remaining);
+ // Only use async-signal-safe operations here
+ self.terminate_signal_received_.store(true, std::memory_order_relaxed);
+
+ // Increment interrupt count atomically for force-quit detection
+ int count = self.interrupt_count_.fetch_add(1, std::memory_order_relaxed) + 1;
+
+ if (count >= force_quit_threshold) {
+ // _Exit is async-signal-safe
+ std::_Exit(128 + SIGINT);
+ }
+
+ // Use write() instead of fprintf - it's async-signal-safe
+ const char msg[] = "Interrupt received. Stopping solver...\n";
+ write(STDERR_FILENO, msg, sizeof(msg) - 1);
}
+
+ // Add atomic counter for simpler force-quit detection
+ std::atomic<int> interrupt_count_{0};
🤖 Prompt for AI Agents |
||
|
|
||
| std::mutex mutex_; | ||
| std::unordered_map<size_t, std::function<void()>> callbacks_; | ||
| std::atomic<bool> terminate_signal_received_{false}; | ||
| size_t next_id_{0}; | ||
| std::deque<time_point> interrupt_times_; | ||
| void (*previous_handler_)(int) = SIG_ERR; | ||
| }; | ||
|
|
||
| } // namespace cuopt | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| /* clang-format off */ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
| /* clang-format on */ | ||
|
|
@@ -3688,13 +3688,13 @@ lp_status_t barrier_solver_t<i_t, f_t>::solve(f_t start_time, | |
| data.cusparse_y_residual_ = data.cusparse_view_.create_vector(data.d_y_residual_); | ||
| data.restrict_u_.resize(num_upper_bounds); | ||
|
|
||
| if (settings.check_termination(start_time)) { | ||
| settings.log.printf("Barrier time limit exceeded\n"); | ||
| return lp_status_t::TIME_LIMIT; | ||
| } | ||
|
|
||
| i_t initial_status = initial_point(data); | ||
| if (settings.check_termination(start_time)) { | ||
| settings.log.printf("Barrier time limit exceeded\n"); | ||
| return lp_status_t::TIME_LIMIT; | ||
| } | ||
|
|
@@ -3793,7 +3793,7 @@ lp_status_t barrier_solver_t<i_t, f_t>::solve(f_t start_time, | |
| while (iter < iteration_limit) { | ||
| raft::common::nvtx::range fun_scope("Barrier: iteration"); | ||
|
|
||
| if (settings.check_termination(start_time)) { | ||
| settings.log.printf("Barrier time limit exceeded\n"); | ||
| return lp_status_t::TIME_LIMIT; | ||
| } | ||
|
|
@@ -3829,7 +3829,7 @@ lp_status_t barrier_solver_t<i_t, f_t>::solve(f_t start_time, | |
| relative_complementarity_residual, | ||
| solution); | ||
| } | ||
| if (settings.check_termination(start_time)) { | ||
| settings.log.printf("Barrier time limit exceeded\n"); | ||
| return lp_status_t::TIME_LIMIT; | ||
| } | ||
|
|
@@ -3869,7 +3869,7 @@ lp_status_t barrier_solver_t<i_t, f_t>::solve(f_t start_time, | |
| } | ||
| data.has_factorization = false; | ||
| data.has_solve_info = false; | ||
| if (settings.check_termination(start_time)) { | ||
| settings.log.printf("Barrier time limit exceeded\n"); | ||
| return lp_status_t::TIME_LIMIT; | ||
| } | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Add Doxygen documentation for new public API methods.
The new
set_lp_callbackandget_lp_callbacksmethods lack documentation comments. Public APIs in headers undercpp/include/cuopt/should include Doxygen-style documentation covering:As per coding guidelines for public header files.
📋 Suggested documentation template
🤖 Prompt for AI Agents