fix(webhook): avoid lock-held awaits in server lifecycle paths

[G7CNF]

Summary

• avoid holding tokio::sync::Mutex<WebhookServer> across async await points during shutdown
• add WebhookServer::begin_shutdown() to split sync state extraction from async shutdown waits
• keep the existing two-phase SIGHUP restart flow lock-safe and use the new lifecycle helper

Why

Issue #869 reports lock guards being held across async I/O boundaries in webhook paths, which can block concurrent webhook processing or graceful shutdown.

Testing

• cargo fmt --all
• cargo test -q shutdown_completes_while_process_message_blocked --package ironclaw
• cargo test -q test_restart_with_addr_rebinds_listener --package ironclaw

Closes #869

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical issue where tokio::sync::Mutex guards were held across asynchronous await points during the webhook server's lifecycle, particularly during shutdown. By introducing a new begin_shutdown method, the PR refactors the shutdown process to extract necessary shutdown primitives while the mutex is held, then releases the mutex before performing any asynchronous waits. This change significantly improves the robustness of the webhook server by preventing potential deadlocks and ensuring concurrent webhook processing and graceful shutdown operations are not blocked.

Highlights

• Refactored Webhook Server Shutdown: Introduced a new begin_shutdown method in WebhookServer to separate the extraction of shutdown primitives from the asynchronous waiting process.
• Avoided Lock-Held Awaits: Modified the webhook server shutdown path to prevent holding tokio::sync::Mutex<WebhookServer> across await points, addressing potential deadlocks and improving concurrency.
• Improved Graceful Shutdown: Enhanced the graceful shutdown mechanism for the webhook server by ensuring locks are released before performing asynchronous waits, resolving issues reported in [CRITICAL] Lock held across async I/O boundary blocks webhook processing #869.

Changelog

• src/channels/webhook_server.rs
  • Added a new public function begin_shutdown to WebhookServer that takes ownership of the shutdown_tx and handle fields.
  • Modified the shutdown async function to call begin_shutdown and then perform the asynchronous wait.
• src/main.rs
  • Updated the main application's shutdown sequence for the webhook server to explicitly acquire the mutex, call begin_shutdown on the WebhookServer instance, and then release the mutex before awaiting the server's shutdown handle.

Activity

• No human activity has been recorded on this pull request yet.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

The changes correctly address the issue of lock-held awaits by introducing a begin_shutdown method to synchronously extract shutdown primitives. This allows asynchronous waiting for the server task to complete to occur outside of the WebhookServer mutex, thereby improving concurrency and preventing potential deadlocks during shutdown and restart operations. The refactoring in main.rs effectively utilizes this new pattern.

[zmanian]

Review: Extract shutdown primitives before dropping mutex lock

Clean fix for a classic Tokio anti-pattern. Holding a mutex across .await (the server handle join) could deadlock if any other task needs the lock during shutdown.

Positives:

• begin_shutdown() cleanly separates handle extraction from async shutdown work
• Take-once semantics ensure idempotent shutdown
• Test verifies second call returns None
• main.rs updated to use the lock-free pattern

LGTM.