Skip to content

8001: Surface errors from SO_ERROR on recv for UDP sockets on Linux #90

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.

Sign up for GitHub

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

Open
martin-augment wants to merge 3 commits into
base: master
Choose a base branch
from
Open

8001: Surface errors from SO_ERROR on recv for UDP sockets on Linux #90

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a1c343f
Add test for expected behavior
SRv6d Mar 31, 2026
6c2a446
Register `Interest::ERROR` in `UdpSocket` `recv`
SRv6d Apr 1, 2026
4f3040c
Add CLAUDE.md & AGENTS.md
martin-g Apr 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .cursor/rules.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
The AI agents should never link to any issue or a pull request
in any GitHub repository in the code reviews!

The AI agents should not review AI agents' config files like CLAUDE.md or AGENTS.md!

5 changes: 5 additions & 0 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
The AI agents should never link to any issue or a pull request
in any GitHub repository in the code reviews!

The AI agents should not review AI agents' config files like CLAUDE.md or AGENTS.md!

5 changes: 5 additions & 0 deletions CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
The AI agents should never link to any issue or a pull request
in any GitHub repository in the code reviews!

The AI agents should not review AI agents' config files like CLAUDE.md or AGENTS.md!

2 changes: 1 addition & 1 deletion tokio/src/net/udp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,7 @@ impl UdpSocket {
pub async fn recv(&self, buf: &mut [u8]) -> io::Result<usize> {
self.io
.registration()
.async_io(Interest::READABLE, || self.io.recv(buf))
.async_io(Interest::READABLE | Interest::ERROR, || self.io.recv(buf))
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

While adding Interest::ERROR is intended to surface SO_ERROR events (such as ICMP unreachable) on connected UDP sockets, this change as implemented has two significant issues:

  1. Broken Readiness Logic: In tokio/src/io/interest.rs, the Interest::mask method uses a match statement that does not handle bitwise combinations. Combining Interest::READABLE | Interest::ERROR will result in Ready::EMPTY being returned by mask(). This will cause async_io to hang indefinitely because the underlying readiness future will be waiting for an empty set of events. A fix for Interest::mask in interest.rs is required to support combined interests.

  2. Inconsistency: This change only updates the recv method. Other receive methods such as recv_from, recv_buf, recv_buf_from, and their try_* counterparts (e.g., try_recv, try_recv_from) still use Interest::READABLE only. This leads to inconsistent behavior across the API where some methods surface errors while others do not.

Copy link
Copy Markdown
Owner Author

@martin-augment martin-augment Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

value:useful; category:bug; feedback: The Gemini AI reviewer is correct! The issue is fixed for only one of the recv_** methods. There are few more similar methods which will need the same fix. Prevents fixing an issue only partially

Copy link
Copy Markdown

@cursor cursor bot Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent Interest::ERROR across similar recv methods

Medium Severity

Interest::ERROR was added to recv but not to the other async receive methods — recv_buf, recv_from, and recv_buf_from — which all use async_io(Interest::READABLE, ...) with the same underlying recv syscalls. This means SO_ERROR (e.g. ConnectionRefused) will only be surfaced when callers use recv, but not when they use any of the other receive variants, creating inconsistent behavior across functionally equivalent APIs.

Additional Locations (2)
Fix in Cursor Fix in Web

Copy link
Copy Markdown
Owner Author

@martin-augment martin-augment Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

value:useful; category:bug; feedback: The Bugbot AI reviewer is correct! The issue is fixed for only one of the recv_** methods. There are few more similar methods which will need the same fix. Prevents fixing an issue only partially

Copy link
Copy Markdown

@augmentcode augmentcode bot Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tokio/src/net/udp.rs:788: This adds Interest::ERROR for recv, but other connected receive APIs like recv_buf (and the poll-based poll_recv / poll_recv_ready paths) still appear to wait only on readable readiness. If they can also observe an SO_ERROR-only readiness state, they may still hang or delay surfacing the error.

Severity: medium

Other Locations
  • tokio/src/net/udp.rs:989
  • tokio/src/net/udp.rs:744
  • tokio/src/net/udp.rs:815

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Copy link
Copy Markdown
Owner Author

@martin-augment martin-augment Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

value:useful; category:bug; feedback: The Augment AI reviewer is correct! The issue is fixed for only one of the recv_** methods. There are few more similar methods which will need the same fix. Prevents fixing an issue only partially

.await
}

Expand Down
23 changes: 22 additions & 1 deletion tokio/tests/udp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
use std::future::poll_fn;
use std::io;
use std::sync::Arc;
use tokio::{io::ReadBuf, net::UdpSocket};
use std::time::Duration;
use tokio::{io::ReadBuf, net::UdpSocket, time};
use tokio_test::assert_ok;

const MSG: &[u8] = b"hello";
Expand Down Expand Up @@ -46,6 +47,26 @@ async fn send_recv_poll() -> std::io::Result<()> {
Ok(())
}

#[tokio::test]
async fn send_to_recv_closed_err_kind_refused() -> std::io::Result<()> {
Copy link
Copy Markdown

@augmentcode augmentcode bot Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tokio/tests/udp.rs:51: This test asserts ErrorKind::ConnectionRefused, but UDP ICMP/error reporting (and the mapped ErrorKind) varies across platforms, and the PR description/title suggests the behavior is Linux-specific. Without a platform cfg, this may be flaky or fail on non-Linux CI.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Copy link
Copy Markdown
Owner Author

@martin-augment martin-augment Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

value:useful; category:bug; feedback: The Augment AI reviewer is correct! The "connection refused" error is Unix specific. Windows uses ConnectionReset instead. The test should improved to expect the correct error for the respective OS. Prevents failed CI due to test failures on Windows and WASM.

let sender = UdpSocket::bind("127.0.0.1:0").await?;
let receiver = UdpSocket::bind("127.0.0.1:0").await?;

let receiver_addr = receiver.local_addr()?;
drop(receiver);
sender.connect(receiver_addr).await?;
sender.send(MSG).await?;

let mut recv_buf = [0u8; 32];
let err = time::timeout(Duration::from_secs(5), sender.recv(&mut recv_buf))
.await
.expect("timed out instead of returning error")
.unwrap_err();

assert_eq!(err.kind(), io::ErrorKind::ConnectionRefused);
Ok(())
}
Comment on lines +51 to +68
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The new test case send_to_recv_closed_err_kind_refused is valuable for verifying that errors are surfaced. However, to ensure consistency and prevent regressions, you should add similar test cases for other receive methods, such as recv_from, try_recv, and recv_buf. This is especially important given that these methods currently lack the Interest::ERROR flag in the implementation and would likely fail this test or time out.

Copy link
Copy Markdown
Owner Author

@martin-augment martin-augment Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

value:useful; category:bug; feedback: The Gemini AI reviewer is correct! The issue is fixed for only one of the recv_** methods. There are few more similar methods which will need the same fix. Prevents fixing an issue only partially

Comment on lines +50 to +68
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Gate this regression test to Linux to avoid cross-platform failures.

Line [50] currently enables this test everywhere, but the asserted behavior (Line [66], ConnectionRefused surfaced on recv) is Linux-specific per the PR intent. On non-Linux targets, this can fail or be flaky.

Suggested fix 
-#[tokio::test]
+#[cfg(target_os = "linux")]
+#[tokio::test]
 async fn send_to_recv_closed_err_kind_refused() -> std::io::Result<()> {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#[tokio::test]
async fn send_to_recv_closed_err_kind_refused() -> std::io::Result<()> {
let sender = UdpSocket::bind("127.0.0.1:0").await?;
let receiver = UdpSocket::bind("127.0.0.1:0").await?;
let receiver_addr = receiver.local_addr()?;
drop(receiver);
sender.connect(receiver_addr).await?;
sender.send(MSG).await?;
let mut recv_buf = [0u8; 32];
let err = time::timeout(Duration::from_secs(5), sender.recv(&mut recv_buf))
.await
.expect("timed out instead of returning error")
.unwrap_err();
assert_eq!(err.kind(), io::ErrorKind::ConnectionRefused);
Ok(())
}
#[cfg(target_os = "linux")]
#[tokio::test]
async fn send_to_recv_closed_err_kind_refused() -> std::io::Result<()> {
let sender = UdpSocket::bind("127.0.0.1:0").await?;
let receiver = UdpSocket::bind("127.0.0.1:0").await?;
let receiver_addr = receiver.local_addr()?;
drop(receiver);
sender.connect(receiver_addr).await?;
sender.send(MSG).await?;
let mut recv_buf = [0u8; 32];
let err = time::timeout(Duration::from_secs(5), sender.recv(&mut recv_buf))
.await
.expect("timed out instead of returning error")
.unwrap_err();
assert_eq!(err.kind(), io::ErrorKind::ConnectionRefused);
Ok(())
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tokio/tests/udp.rs` around lines 50 - 68, The test
send_to_recv_closed_err_kind_refused exhibits Linux-specific behavior and must
be gated to Linux; add a platform cfg attribute (e.g. #[cfg(target_os =
"linux")] or #[cfg(all(unix, target_os = "linux"))]) directly above the
#[tokio::test] for the send_to_recv_closed_err_kind_refused function so the test
only runs on Linux targets, leaving the test body and assertions unchanged.


#[tokio::test]
async fn send_to_recv_from() -> std::io::Result<()> {
let sender = UdpSocket::bind("127.0.0.1:0").await?;
Expand Down
Loading