Skip to content

Conversation

maddeleine
Copy link
Contributor

@maddeleine maddeleine commented Jul 18, 2025

Release Summary:

Adds unstable feature to move s2n-tls operations out of the main event loop.

Resolved issues:

resolves #2598

Description of changes:

Adds a new TLS provider that spawns an asynchronous task which will complete the TLS handshake.
I am closing #2688 in favor of this PR.

Call-outs:

Testing:

New integ tests.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@maddeleine maddeleine requested review from WesleyRosenblum and camshaft and removed request for WesleyRosenblum July 21, 2025 16:29
Comment on lines +152 to +207
let cloned_waker = &context.waker().clone();
let mut ctx = core::task::Context::from_waker(cloned_waker);
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you need to clone this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am using context to both call the Context trait methods(e.g. context.on_handshake_keys()) and in the poll_slice function in line 155. Essentially, I can't borrow the context twice if one of the borrows in mutable(which it is.) So I'm cloning this to get around it.

Comment on lines 113 to 147
if let Poll::Ready(Ok(mut slice)) = send_to_quic.poll_slice(ctx) {
match res {
Ok(()) => {
let _ = slice.push(Msg::TlsDone);
}

Err(e) => {
let _ = slice.push(Msg::TlsError(e));
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a little worried about messages getting dropped... But I guess it would only happen if the TLS implementation sends a lot of messages at once.

@maddeleine maddeleine requested review from camshaft and removed request for WesleyRosenblum and camshaft July 29, 2025 03:43
camshaft
camshaft previously approved these changes Jul 31, 2025
Copy link
Contributor

@camshaft camshaft left a comment

Choose a reason for hiding this comment

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

Everything looks good other than the clippy warning and the commented out thing!

@maddeleine maddeleine merged commit 2af1b16 into aws:main Jul 31, 2025
122 checks passed
@maddeleine maddeleine deleted the second_try branch July 31, 2025 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Offload the TLS crypto operations to separate threads
2 participants