-
Notifications
You must be signed in to change notification settings - Fork 9
Implement UdpStream::poll_shutdown instead of UdpStream::shutdown #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Please publish this patch. |
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.
Pull Request Overview
This pull request refines the UDP stream code by removing redundant shutdown functionality and updating the UdpListener::accept method to streamline return values. Key changes include removing UdpStream::shutdown, updating UdpListener::accept to return only a UdpStream, and updating dependency versions in Cargo.toml and related examples.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/lib.rs | Updated UdpListener::accept return type and removed UdpStream::shutdown. |
| rustfmt.toml | Configured max_width to 140. |
| examples/echo-udp.rs | Updated code to match new accept return type. |
| examples/echo-dtls.rs | Updated code to match new accept return type. |
| README.md | Updated dependency version demonstration. |
| Cargo.toml | Bumped library version and env_logger dependency version. |
Co-authored-by: Copilot <[email protected]>
|
Let’s keep the accept function signature as is to maintain consistency with TcpListener::accept. For poll_shutdown, since drop is an async function, let’s invoke it asynchronously. |
8754333 to
83bbc30
Compare
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.
Pull Request Overview
This PR removes the deprecated UdpStream::shutdown method and implements UdpStream::poll_shutdown to consolidate shutdown logic while updating related return types and error handling.
- Replaces UdpStream::shutdown with an asynchronous poll_shutdown implementation.
- Updates usage of std::io::Result across functions and adjusts error handling in stream operations.
- Bumps the package version and updates auxiliary configuration and examples to reflect changes.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/lib.rs | Refactors shutdown handling for UdpStream and updates error conversions. |
| rustfmt.toml | Sets max_width to 140. |
| examples/echo-udp.rs | Adapts to the new poll_shutdown implementation with updated error handling. |
| examples/echo-dtls.rs | Adjusts error conversion and shutdown calls to work with poll_shutdown. |
| README.md | Updates dependency version and code snippets. |
| Cargo.toml | Increments version to 0.1.0 and bumps env_logger dependency. |
Comments suppressed due to low confidence (1)
src/lib.rs:274
- [nitpick] Using self.drop.take() ensures that the shutdown logic only runs once; please confirm that this idempotence is the intended behavior for subsequent shutdown calls.
if let Some(drop) = self.drop.take() {
|
how about change this Lines 281 to 292 in 465c362
to fn poll_write(self: Pin<&mut Self>, cx: &mut Context, buf: &[u8]) -> Poll<std::io::Result<usize>> {
self.socket.poll_send_to(cx, buf, self.peer_addr)
} |
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.
Pull Request Overview
This PR removes the legacy UdpStream::shutdown method and implements UdpStream::poll_shutdown, while also updating error types to std::io::Result and making minor dependency/documentation updates.
- Removed the shutdown workaround in favor of using poll_shutdown in AsyncWrite.
- Updated return types from io::Result to std::io::Result throughout the API and examples.
- Bumped version numbers and adjusted formatting in Cargo.toml, rustfmt.toml, and README.md.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/lib.rs | Updated error types, removed shutdown(), and implemented poll_shutdown with drop handling changes. |
| rustfmt.toml | Updated max_width setting. |
| examples/echo-udp.rs | Adapted to changes in shutdown API and error type adjustments. |
| examples/echo-dtls.rs | Adapted DTLS echo example to use poll_shutdown via AsyncWrite. |
| README.md | Updated dependency version and code block annotations. |
| Cargo.toml | Bumped library version and updated dependency versions. |
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.
Pull Request Overview
This PR refactors the UDP stream shutdown mechanism by removing the old UdpStream::shutdown API and consolidating its functionality into the AsyncWrite::poll_shutdown implementation. Key changes include refactoring error types to use std::io, adding a drop_sent flag to prevent duplicate drop notifications, and updating examples and documentation to reflect these API changes.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/lib.rs | Refactors UdpStream shutdown handling and adds drop_sent tracking |
| rustfmt.toml | Updates maximum line width configuration |
| examples/echo-udp.rs | Adjusts error handling and shutdown call in the echo example |
| examples/echo-dtls.rs | Updates DTLS echo example for error propagation and shutdown call |
| README.md | Updates dependency version and code snippet annotations |
| Cargo.toml | Bumps version and updates dependency versions |
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.
Pull Request Overview
This PR removes the UdpStream::shutdown method and implements proper shutdown handling via UdpStream::poll_shutdown while updating error types and dependency version bumps.
- Removed UdpStream::shutdown and integrated its logic into poll_shutdown
- Updated various usages from io to std::io and improved error handling in echo examples
- Adjusted version numbers and formatting configuration
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib.rs | Replaced shutdown with poll_shutdown and refactored drop signal logic |
| rustfmt.toml | Updated max_width configuration |
| examples/echo-udp.rs | Modified error handling and shutdown usage in the echo UDP example |
| examples/echo-dtls.rs | Updated error conversion and shutdown handling in the DTLS example |
| README.md | Updated dependency version and markdown code block syntax |
| Cargo.toml | Bumped version and updated dependency versions |
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.
Pull Request Overview
This PR implements the migration from the removed UdpStream::shutdown method to the new poll_shutdown implementation while refactoring the shutdown logic.
- Update UdpStream to use a drop_sent flag and a send_drop_helper method for managing shutdown behavior.
- Modify examples to reflect type changes and the new shutdown API.
- Update Cargo.toml and rustfmt.toml for version and configuration changes.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib.rs | Replace UdpStream::shutdown with a new poll_shutdown implementation and integrate send_drop_helper for shutdown signaling. |
| rustfmt.toml | Configure max_width to 140 to adjust formatting style. |
| examples/echo-udp.rs | Update error handling to call the new shutdown method. |
| examples/echo-dtls.rs | Update DTLS example to use the new shutdown approach. |
| README.md | Update documentation and example code to reflect API changes. |
| Cargo.toml | Bump version to 0.1.0 and update dependency versions. |
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
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.
Pull Request Overview
This PR removes the UdpStream::shutdown method and migrates its logic into the AsyncWrite trait’s poll_shutdown implementation. Key changes include replacing shutdown with the new send_drop_helper method, updating error types to use std::io::Result, and minor updates in examples and Cargo.toml to reflect version and dependency changes.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/lib.rs | Removed UdpStream::shutdown, introduced send_drop_helper, and updated method signatures |
| rustfmt.toml | Updated formatting configuration (max_width set to 140) |
| examples/echo-udp.rs | Updated error handling and use of shutdown in the echo example |
| examples/echo-dtls.rs | Updated DTLS echo example to use proper error mappings |
| README.md | Updated dependency and usage documentation |
| Cargo.toml | Bumped version and updated dependency versions |
Comments suppressed due to low confidence (1)
src/lib.rs:106
- [nitpick] Consider renaming the 'drop_sent' flag to a more descriptive name such as 'has_sent_drop' to improve code clarity.
drop_sent: false,
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.
Pull Request Overview
This PR removes the custom UdpStream::shutdown method and implements poll_shutdown instead, consolidating shutdown logic into a single helper.
- Refactors shutdown behavior using a new send_drop_helper and poll_shutdown implementation.
- Updates error types from io::Error to std::io::Error for consistency.
- Adjusts examples, Cargo.toml, and formatting configuration accordingly.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/lib.rs | Replaces UdpStream::shutdown with poll_shutdown and adds send_drop_helper to ensure drop logic is only executed once. |
| rustfmt.toml | Adds a max_width configuration. |
| examples/echo-udp.rs | Updates error handling for stream read timeout and shutdown invocation. |
| examples/echo-dtls.rs | Adjusts error handling and shutdown flow with std::io::Error conversions. |
| README.md | Updates dependency version and code snippet annotations. |
| Cargo.toml | Bumps version and updates dependency for env_logger. |
Broken changes:
UdpStream::shutdown, because there existUdpStream::poll_shutdownand all logic are merged into.