7855: Add AbortOnDrop complement type to AbortOnDropHandle

[martin-augment]

7855: To review by AI

[coderabbitai]

Walkthrough

The pull request introduces a new public struct AbortOnDrop in the task module that wraps Tokio's AbortHandle. This new type implements a Drop trait that automatically aborts the associated task when the wrapper is dropped. The AbortOnDrop struct provides methods: new() for construction, abort() for manual task termination, is_finished() to check task completion status, and detach() to retrieve the underlying AbortHandle without triggering automatic abort. A Debug implementation is also included. The public API is extended by re-exporting the new type through the task module, and tests are updated to exercise the new functionality.

✨ Finishing touches

• 📝 Generate docstrings

---

🧹 Recent nitpick comments

tokio-util/src/task/abort_on_drop.rs (2)

85-100: Solid implementation following established patterns.

The struct definition and Drop implementation correctly mirror AbortOnDropHandle. The #[must_use] attribute appropriately warns users about the drop-abort behavior.

Minor: There's a double blank line at line 91 that could be consolidated to a single blank line for consistency.

---

120-127: The detach implementation is correct but could potentially be simplified.

The current unsafe implementation is sound and matches AbortOnDropHandle. However, since AbortHandle implements Clone, you could consider a simpler safe implementation:

pub fn detach(self) -> AbortHandle {
    let handle = self.0.clone();
    std::mem::forget(self);
    handle
}

That said, the current approach avoids the clone and is consistent with AbortOnDropHandle, so keeping it as-is is also reasonable.

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 954990b and 6be1ab6.

📒 Files selected for processing (3)

• tokio-util/src/task/abort_on_drop.rs
• tokio-util/src/task/mod.rs
• tokio-util/tests/abort_on_drop.rs

🧰 Additional context used

🧬 Code graph analysis (2)

tokio-util/src/task/abort_on_drop.rs (1)

tokio-util/src/task/join_queue.rs (2)

• new (28-30)
• fmt (382-386)

tokio-util/tests/abort_on_drop.rs (1)

tokio-util/src/task/abort_on_drop.rs (6)

• new (31-33)
• new (104-106)
• drop (24-26)
• drop (97-99)
• is_finished (43-45)
• is_finished (116-118)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Cursor Bugbot

🔇 Additional comments (8)

tokio-util/src/task/abort_on_drop.rs (3)

3-5: LGTM! Doc comment correctly describes the new type.

The documentation appropriately extends the module description to cover AbortOnDrop alongside AbortOnDropHandle.

---

130-136: LGTM!

The Debug implementation follows the same pattern as AbortOnDropHandle and correctly displays the task ID for debugging purposes.

---

148-151: LGTM!

Good addition to verify that AbortOnDrop implements Debug, ensuring compile-time verification of the trait bound.

tokio-util/src/task/mod.rs (1)

14-15: LGTM!

The re-export correctly exposes AbortOnDrop as part of the public API alongside AbortOnDropHandle.

tokio-util/tests/abort_on_drop.rs (4)

1-2: LGTM!

Import correctly updated to include the new AbortOnDrop type.

---

41-51: LGTM!

Test correctly verifies that dropping AbortOnDrop aborts the associated task. The oneshot channel pattern effectively detects task termination.

---

53-64: LGTM!

Test correctly verifies explicit abort() call functionality and the is_finished() status check.

---

66-76: LGTM!

Test correctly verifies that detach() prevents the automatic abort-on-drop behavior. The yield_now() ensures the scheduler has time to process any would-be aborts before asserting the task is still alive.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[claude]

Pull Request Review: Add AbortOnDrop complement type to AbortOnDropHandle

Summary

This PR adds a new AbortOnDrop type that wraps AbortHandle and aborts the task on drop, complementing the existing AbortOnDropHandle which wraps JoinHandle. The implementation is clean and follows the established pattern in the codebase.

Code Quality ✅

Strengths:

• Consistent API design: The new AbortOnDrop type mirrors the API of AbortOnDropHandle, making it intuitive for users
• Proper use of ManuallyDrop: The detach() implementation correctly uses ManuallyDrop and ptr::read to avoid invoking the Drop impl
• Good documentation: Clear doc comments explaining the difference between AbortOnDrop and AbortOnDropHandle
• Appropriate #[must_use] attribute: Correctly warns users that dropping the handle has side effects

Minor suggestions:

1. Grammar issue in documentation (tokio-util/src/task/abort_on_drop.rs:88-89):

   • Line 88: "tasks completion" should be "task's completion" (possessive)

   /// Unlike [`AbortOnDropHandle`], [`AbortOnDrop`] does not represent the permission to await a
   /// task's completion, and in exchange can be a concrete type.
   

2. Extra blank line (tokio-util/src/task/abort_on_drop.rs:90-91):

   • There's an unnecessary extra blank line in the doc comment
   • Should be consistent with the single blank line style used elsewhere

Potential Issues 🔍

No significant bugs found. The implementation is solid.

Security Concerns ✅

No security issues identified. The code:

• Uses unsafe appropriately with proper safety documentation
• Doesn't introduce any new unsafe patterns beyond what exists in AbortOnDropHandle
• The ptr::read usage is safe as documented in the safety comment

Performance Considerations ✅

No performance concerns. The implementation:

• Is zero-cost wrapper around AbortHandle
• Uses the same efficient ManuallyDrop + ptr::read pattern as AbortOnDropHandle
• No additional allocations or overhead

Test Coverage ✅

Excellent test coverage. The PR adds three comprehensive tests:

1. ✅ handle_aborts_task_on_drop - Tests basic abort-on-drop behavior
2. ✅ handle_aborts_task_directly - Tests explicit abort() method
3. ✅ handle_does_not_abort_after_detach - Tests detach() prevents abort

These mirror the existing tests for AbortOnDropHandle and cover all public API methods.

Additional test considerations:

• ✅ Debug impl is tested via assert_debug() test
• ✅ All public methods (new, abort, is_finished, detach) are covered

Best Practices ✅

The code follows Rust and Tokio best practices:

• Consistent naming conventions
• Proper use of impl blocks
• Standard library traits appropriately implemented (Debug, Drop)
• Follows the existing pattern established by AbortOnDropHandle

Recommendation

LGTM with minor documentation fixes. This is a well-implemented addition that provides useful functionality. The only issues are cosmetic:

1. Fix the grammar: "tasks completion" → "task's completion"
2. Remove the extra blank line in the doc comment

The implementation is safe, well-tested, and follows established patterns in the codebase.

---

Review generated by Claude Code

[augmentcode]

🤖 Augment PR Summary

Summary: Adds a concrete abort-on-drop handle type for tasks.

Changes:

• Introduces AbortOnDrop, a wrapper around tokio::task::AbortHandle that aborts the task when dropped
• Exports AbortOnDrop from tokio_util::task alongside AbortOnDropHandle
• Adds unit and integration tests covering drop/abort/detach behavior for the new type

Technical Notes: Implements detach() via ManuallyDrop to avoid triggering the drop-abort behavior.

_{🤖 Was this summary useful? React with 👍 or 👎}

[augmentcode]

Review completed. 2 suggestions posted.

Comment augment review to trigger a new review at any time.

[augmentcode]

The rustdoc link target for [aborts] points to tokio::task::JoinHandle::abort, but AbortOnDrop wraps AbortHandle; linking to AbortHandle::abort would keep the docs consistent with the API surface.

_{🤖 Was this useful? React with 👍 or 👎}

[martin-augment]

value:useful; category:documentation; feedback:The Augment AI reviewer is correct! The new AbortOnDrop struct wraps an AbortHandle, not a JoinHandle. The doclink is wrong and it has to be fixed.

[augmentcode]

This inline comment mentions dropping the original join handle, but AbortOnDrop::detach() returns an AbortHandle; it may be worth clarifying the comment to match what’s actually being detached/dropped here.

_{🤖 Was this useful? React with 👍 or 👎}

[martin-augment]

value:useful; category:documentation; feedback:The Augment AI reviewer is correct! The new AbortOnDrop struct wraps an AbortHandle, not a JoinHandle. The comment is wrong and it has to be fixed.

[martin-augment]

Grammar issue in documentation (tokio-util/src/task/abort_on_drop.rs:88-89):

• Line 88: "tasks completion" should be "task's completion" (possessive)

/// Unlike [`AbortOnDropHandle`], [`AbortOnDrop`] does not represent the permission to await a
/// task's completion, and in exchange can be a concrete type.

value:good-to-have; category:documentation; feedback:The Claude AI reviewer is correct! The grammar of the docstring is slightly incorrect and it has to be fixed to prevent confusion in the reader that there are more than one tasks involved.

[martin-augment]

Extra blank line (tokio-util/src/task/abort_on_drop.rs:90-91):

• There's an unnecessary extra blank line in the doc comment
• Should be consistent with the single blank line style used elsewhere

value:good-to-have; category:documentation; feedback:The Claude AI reviewer is correct! The extra empty line in the documentation does not bring any value, nor any harm. It should be removed to make the documentation styling consistent with the rest of the file/repository.