feat: Implement ThreadAware for more more types by AdomasBekeras · Pull Request #569 · microsoft/oxidizer

AdomasBekeras · 2026-07-13T10:04:02Z

Implement ThreadAware for std::net::SocketAddr

Copilot

Pull request overview

This PR expands thread_aware’s coverage of “inert / relocation-no-op” types by adding a feature-gated ThreadAware implementation for tokio::sync::mpsc::UnboundedSender<T> and by adding a no-op impl for std::net::SocketAddr, along with tests and documentation updates.

Changes:

Add ThreadAware no-op relocation support for std::net::SocketAddr (with unit test).
Introduce a new tokio feature that enables a ThreadAware impl for tokio::sync::mpsc::UnboundedSender<T> (plus tests).
Update crate docs/README and metadata (allowed_external_types, dependency wiring) to reflect the new tokio feature.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
crates/thread_aware/src/third_party/tokio.rs	Adds the feature-gated `ThreadAware` impl for `tokio::sync::mpsc::UnboundedSender<T>` and validates behavior via tests.
crates/thread_aware/src/third_party/mod.rs	Registers the new `tokio` third-party module behind `cfg(any(test, feature = "tokio"))` and updates module docs.
crates/thread_aware/src/lib.rs	Documents the new `tokio` opt-in feature in the crate-level feature list.
crates/thread_aware/src/impls.rs	Adds a no-op `ThreadAware` impl for `SocketAddr` and a unit test verifying relocation is a no-op.
crates/thread_aware/README.md	Updates the generated README feature list to include `tokio`.
crates/thread_aware/Cargo.toml	Adds the `tokio` feature, dependency/dev-dependency entries, and allowlists the external Tokio type for external-types checking.
Cargo.lock	Records the new `tokio` dependency for `thread_aware`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

codecov · 2026-07-13T10:24:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.0%. Comparing base (f78524e) to head (7f3c9cf).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff            @@
##             main     #569    +/-   ##
========================================
  Coverage   100.0%   100.0%            
========================================
  Files         356      360     +4     
  Lines       27388    27886   +498     
========================================
+ Hits        27388    27886   +498

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

sandersaares

We need to define clear acceptance criteria in docs/DESIGN.md for why/when it is OK to add such out-of-box ThreadAware implementations. Today this feels like we are just randomly adding stuff here and I do not really see a rationale or a way to review whether something is OK to include or not in this way today. Certainly, including one random piece of Tokio feels weird (why not other pieces?).

@ralfbiedert can you help create these acceptance criteria so we can review this PR and other similar future PRs with some grounding?

Copilot

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.

AdomasBekeras · 2026-07-13T10:51:39Z

We need to define clear acceptance criteria in docs/DESIGN.md for why/when it is OK to add such out-of-box ThreadAware implementations. Today this feels like we are just randomly adding stuff here and I do not really see a rationale or a way to review whether something is OK to include or not in this way today. Certainly, including one random piece of Tokio feels weird (why not other pieces?).

@ralfbiedert can you help create these acceptance criteria so we can review this PR and other similar future PRs with some grounding?

I'm adding it because Ralf asked to add it so I assume it's ok to add it here. I don't have a comment on the acceptance criteria but that's not really in the scope of this PR.

ralfbiedert · 2026-07-13T11:28:54Z

I'm adding it because Ralf asked to add it so I assume it's ok to add it here. I don't have a comment on the acceptance criteria but that's not really in the scope of this PR.

Sorry there was a misunderstanding. Adding ThreadAware for std items is generally fine, adding extra crates not so much. In fact, I'm a bit unhappy we have the existing ones already and we should consider removing them for 1.0 as they must live in the core crate and will be perma stable.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6503296c-52d4-4236-82bb-958903e3e519

AdomasBekeras · 2026-07-13T11:35:34Z

Removed the impl for the channel now

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

sandersaares

I'm adding it because Ralf asked to add it so I assume it's ok to add it here. I don't have a comment on the acceptance criteria but that's not really in the scope of this PR.

This, combined with Ralf's reply, highlights the importance of including acceptance criteria. Let's please add at least a statement into AGENTS.md or DESIGN.md or similar that we can only add implementations for std without special exception here. If you think this is an egregious violation of this PR's scope, I am OK if you just file a work item to add this line later.

ralfbiedert · 2026-07-14T11:11:33Z

        assert_eq!(err_string, Err("error".to_string()));
    }

+    #[test]


These tests are mostly useless and just noise. The only thing that should be asserted for most types is that the upstream type in fact impls it, which in and by itself can be a simple const fn assert_impl_threadware<T>() {}, which in turn violates M-TAUTOLOGICAL-TESTS.

Makes sense, removed

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6503296c-52d4-4236-82bb-958903e3e519

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Implement ThreadAware for more more types

c80aa7d

Copilot AI review requested due to automatic review settings July 13, 2026 10:04

Copilot started reviewing on behalf of AdomasBekeras July 13, 2026 10:08 View session

Copilot AI reviewed Jul 13, 2026

View reviewed changes

AdomasBekeras changed the title ~~Implement ThreadAware for more more types~~ feat: Implement ThreadAware for more more types Jul 13, 2026

spelling

de465dd

Copilot AI review requested due to automatic review settings July 13, 2026 10:26

sandersaares requested changes Jul 13, 2026

View reviewed changes

Copilot started reviewing on behalf of AdomasBekeras July 13, 2026 10:34 View session

Copilot AI reviewed Jul 13, 2026

View reviewed changes

ralfbiedert approved these changes Jul 13, 2026

View reviewed changes

fix(thread_aware): remove tokio implementation

f86e193

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6503296c-52d4-4236-82bb-958903e3e519

Copilot AI review requested due to automatic review settings July 13, 2026 11:34

Copilot started reviewing on behalf of AdomasBekeras July 13, 2026 11:38 View session

Copilot AI reviewed Jul 13, 2026

View reviewed changes

Merge branch 'main' into u/abekeras/thread-aware-types

91e89c9

AdomasBekeras requested review from Copilot and sandersaares July 13, 2026 12:26

Copilot started reviewing on behalf of AdomasBekeras July 13, 2026 12:31 View session

Copilot AI reviewed Jul 13, 2026

View reviewed changes

martintmk approved these changes Jul 13, 2026

View reviewed changes

sandersaares requested changes Jul 13, 2026

View reviewed changes

Vaiz approved these changes Jul 13, 2026

View reviewed changes

ralfbiedert reviewed Jul 14, 2026

View reviewed changes

ralfbiedert approved these changes Jul 14, 2026

View reviewed changes

test(thread_aware): remove redundant socket address test

7f3c9cf

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6503296c-52d4-4236-82bb-958903e3e519

Copilot AI review requested due to automatic review settings July 14, 2026 14:13

Copilot started reviewing on behalf of AdomasBekeras July 14, 2026 14:14 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Uh oh!

Conversation

AdomasBekeras commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

codecov Bot commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sandersaares left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

AdomasBekeras commented Jul 13, 2026

Uh oh!

ralfbiedert commented Jul 13, 2026

Uh oh!

AdomasBekeras commented Jul 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

sandersaares left a comment

Choose a reason for hiding this comment

Uh oh!

ralfbiedert Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

AdomasBekeras Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

AdomasBekeras commented Jul 13, 2026 •

edited

Loading

codecov Bot commented Jul 13, 2026 •

edited

Loading