fix(grpc): terminate workers on server shutdown

[tmckayus]

Fix an oversight on grpc server shutdown. The server would not exit if a worker was running a solve. The original Python REST server killed any running jobs if the server was shutdown and set the job status accordingly. This matches that behavior.

Without this behavior, the process has to be killed from the OS or every running job id has to be explicitly canceled.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Changes

The gRPC server now uses coordinated SIGINT/SIGTERM handling through sigwait, cancels active work, terminates workers, closes pipes, performs bounded shutdown cleanup, and adds integration coverage and documentation for prompt termination.

gRPC shutdown flow

Layer / File(s)	Summary
Shutdown infrastructure cpp/src/grpc/server/grpc_server_types.hpp, cpp/src/grpc/server/grpc_worker_infra.cpp, cpp/src/grpc/server/grpc_server_threads.cpp	Adds synchronized worker PID tracking, worker termination, pipe closure, active-job cancellation, waiter notification, respawn coordination, and bounded worker waits.
Signal shutdown orchestration cpp/src/grpc/server/grpc_server_main.cpp, cpp/src/grpc/server/grpc_worker.cpp	Uses signal masking and sigwait to cancel jobs, terminate workers, close pipes, and stop gRPC with a timed deadline; workers ignore inherited SIGINT and SIGTERM.
Shutdown behavior validation cpp/tests/linear_programming/grpc/grpc_integration_test.cpp, cpp/docs/grpc-server-architecture.md, docs/cuopt/source/cuopt-grpc/grpc-server-architecture.md	Adds prompt-SIGINT integration coverage and documents the coordinated shutdown behavior.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested reviewers: akifcorduk, hlinsen, iroy30

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the main change: terminating gRPC workers during server shutdown.
Description check	✅ Passed	The description is directly related to the shutdown fix and job-cancellation behavior.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cpp/src/grpc/server/grpc_worker_infra.cpp`:
- Around line 211-216: Synchronize all access to worker_pids during shutdown:
update kill_all_workers in cpp/src/grpc/server/grpc_worker_infra.cpp (lines
211-216) to use the same mutex protecting worker_monitor_thread’s reads and
writes, and update the shutdown path in cpp/src/grpc/server/grpc_server_main.cpp
(lines 328-342) to stop and join monitor_thread before invoking kill_all_workers
if that is the chosen strategy. Ensure no worker_pids access occurs concurrently
with shutdown.

In `@cpp/tests/linear_programming/grpc/grpc_integration_test.cpp`:
- Around line 1700-1733: Update SigintDuringRunningJobShutsDownPromptly to poll
the server process’s actual exit status with waitpid(..., WNOHANG) instead of
relying on server_.is_running(), while preserving the 15-second timeout and
failure message. Ensure the child process is reaped and the test exits the loop
when it has terminated, including zombie states.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ff657884-bd78-4ba5-bbfa-6e9ac4c9e81c

📥 Commits

Reviewing files that changed from the base of the PR and between 7080eca and 58dd2aa.

📒 Files selected for processing (7)

• cpp/docs/grpc-server-architecture.md
• cpp/src/grpc/server/grpc_server_main.cpp
• cpp/src/grpc/server/grpc_server_types.hpp
• cpp/src/grpc/server/grpc_worker.cpp
• cpp/src/grpc/server/grpc_worker_infra.cpp
• cpp/tests/linear_programming/grpc/grpc_integration_test.cpp
• docs/cuopt/source/cuopt-grpc/grpc-server-architecture.md

[github-actions]

CI Test Summary

✅ All 0 test job(s) passed.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

cpp/src/grpc/server/grpc_worker_infra.cpp (1)

247-260: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Gate WaitForCompletion() on shutdown state. cancel_all_active_jobs_for_shutdown() clears waiting_threads, but a late RPC can still create a fresh JobWaiter afterward and miss the shutdown notification. Check shm_ctrl->shutdown_requested under waiters_mutex and return the shutdown error immediately.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/src/grpc/server/grpc_worker_infra.cpp` around lines 247 - 260, Update
WaitForCompletion() to check shm_ctrl->shutdown_requested while holding
waiters_mutex before creating or registering a new JobWaiter. If shutdown is
requested, return the existing “Server shutting down” error immediately and
avoid adding the waiter to waiting_threads, preserving
cancel_all_active_jobs_for_shutdown() behavior for already-registered waiters.

Source: Path instructions

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@cpp/src/grpc/server/grpc_worker_infra.cpp`:
- Around line 247-260: Update WaitForCompletion() to check
shm_ctrl->shutdown_requested while holding waiters_mutex before creating or
registering a new JobWaiter. If shutdown is requested, return the existing
“Server shutting down” error immediately and avoid adding the waiter to
waiting_threads, preserving cancel_all_active_jobs_for_shutdown() behavior for
already-registered waiters.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 278ac4c5-0794-439f-91a0-ad6a43fb3e08

📥 Commits

Reviewing files that changed from the base of the PR and between 58dd2aa and d2ccf81.

📒 Files selected for processing (4)

• cpp/docs/grpc-server-architecture.md
• cpp/src/grpc/server/grpc_server_main.cpp
• cpp/src/grpc/server/grpc_server_types.hpp
• cpp/src/grpc/server/grpc_worker_infra.cpp

🚧 Files skipped from review as they are similar to previous changes (3)

• cpp/docs/grpc-server-architecture.md
• cpp/src/grpc/server/grpc_server_main.cpp
• cpp/src/grpc/server/grpc_server_types.hpp

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cpp/src/grpc/server/grpc_server_main.cpp`:
- Around line 197-199: Update the error handling in the pthread_sigmask call to
capture its direct return code and pass that code to strerror, rather than using
errno. Preserve the existing SERVER_LOG_ERROR message context and return value.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b7a209e6-7e41-4525-9a54-09c9c1606452

📥 Commits

Reviewing files that changed from the base of the PR and between d2ccf81 and 38ca744.

📒 Files selected for processing (3)

• cpp/docs/grpc-server-architecture.md
• cpp/src/grpc/server/grpc_server_main.cpp
• cpp/src/grpc/server/grpc_server_types.hpp

🚧 Files skipped from review as they are similar to previous changes (1)

• cpp/docs/grpc-server-architecture.md

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

cpp/src/grpc/server/grpc_worker_infra.cpp (1)

202-210: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Keep worker_pids aligned with worker ids. spawn_workers() appends only successful PIDs, so a startup failure compacts the vector and shifts later entries. worker_monitor_thread() then treats the vector index as the worker id when it respawns (spawn_single_worker(dw.index)), which can restart the wrong worker/pipe slot. Pre-size worker_pids to config.num_workers and assign by i, or store the worker id with each PID.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/src/grpc/server/grpc_worker_infra.cpp` around lines 202 - 210, Update
spawn_workers() so worker_pids preserves worker-id indexing: pre-size it to
config.num_workers and assign each successful PID at index i rather than
appending. Keep failed startup entries in their corresponding slots so
worker_monitor_thread() and spawn_single_worker(dw.index) continue targeting the
correct worker and pipe.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cpp/tests/linear_programming/grpc/grpc_integration_test.cpp`:
- Around line 1750-1753: Replace the fixed two-second sleep in the SIGINT
shutdown test with bounded polling through check_status(), waiting until the job
is confirmed to be PROCESSING. Assert that this state is reached before
signaling the server, while retaining the existing wait_exited assertion to
validate prompt shutdown during an active job.
- Around line 201-227: Update wait_exited so it checks the result of reap_group
before calling clear_lifecycle_state. Only clear lifecycle state and return
success when the worker process group is fully reaped; otherwise preserve the
group state for TearDown’s server_.stop() to clean up and return failure.

---

Outside diff comments:
In `@cpp/src/grpc/server/grpc_worker_infra.cpp`:
- Around line 202-210: Update spawn_workers() so worker_pids preserves worker-id
indexing: pre-size it to config.num_workers and assign each successful PID at
index i rather than appending. Keep failed startup entries in their
corresponding slots so worker_monitor_thread() and spawn_single_worker(dw.index)
continue targeting the correct worker and pipe.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 667172ec-65cf-4d8f-bbba-7b5c13fc4101

📥 Commits

Reviewing files that changed from the base of the PR and between 38ca744 and 5ae595c.

📒 Files selected for processing (5)

• cpp/src/grpc/server/grpc_server_main.cpp
• cpp/src/grpc/server/grpc_server_threads.cpp
• cpp/src/grpc/server/grpc_server_types.hpp
• cpp/src/grpc/server/grpc_worker_infra.cpp
• cpp/tests/linear_programming/grpc/grpc_integration_test.cpp

🚧 Files skipped from review as they are similar to previous changes (2)

• cpp/src/grpc/server/grpc_server_types.hpp
• cpp/src/grpc/server/grpc_server_main.cpp

[tmckayus]

/ok to test 5ae595c

[tmckayus]

/ok to test bcddd9d

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cpp/src/grpc/server/grpc_server_threads.cpp`:
- Around line 70-72: Update the worker respawn handling around
spawn_single_worker so that failures where new_pid <= 0 emit an error log
identifying the affected worker slot and the failed respawn; preserve the
existing success log for new_pid > 0.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 41e6bcf6-1a6b-4869-a12e-5dfd661dbf2f

📥 Commits

Reviewing files that changed from the base of the PR and between 5ae595c and bcddd9d.

📒 Files selected for processing (5)

• cpp/src/grpc/server/grpc_server_main.cpp
• cpp/src/grpc/server/grpc_server_threads.cpp
• cpp/src/grpc/server/grpc_server_types.hpp
• cpp/src/grpc/server/grpc_worker_infra.cpp
• cpp/tests/linear_programming/grpc/grpc_integration_test.cpp

🚧 Files skipped from review as they are similar to previous changes (4)

• cpp/src/grpc/server/grpc_server_main.cpp
• cpp/src/grpc/server/grpc_server_types.hpp
• cpp/tests/linear_programming/grpc/grpc_integration_test.cpp
• cpp/src/grpc/server/grpc_worker_infra.cpp

[tmckayus]

thanks to @akifcorduk for suggesting watchdog time expansion