Skip to content
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

Add generic trait to combine UnixListener and TcpListener #4385

Merged
merged 17 commits into from Jan 27, 2022
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions tokio-util/src/lib.rs
Expand Up @@ -30,6 +30,7 @@ cfg_codec! {

cfg_net! {
pub mod udp;
pub mod net;
}

cfg_compat! {
Expand Down
104 changes: 104 additions & 0 deletions tokio-util/src/net/mod.rs
@@ -0,0 +1,104 @@
//! TCP/UDP/Unix helpers for tokio.

use crate::either::Either;
use std::future::Future;
use std::io::Result;
use std::pin::Pin;
use std::task::{Context, Poll};

#[cfg(unix)]
pub mod unix;

/// A trait for a listener: `TcpListener` and `UnixListener`.
pub trait Listener {
/// The stream's type of this listener.
type Io: tokio::io::AsyncRead + tokio::io::AsyncWrite;
/// The socket address type of this listener.
type Addr;

/// Polls to accept a new incoming connection to this listener.
fn poll_accept(&mut self, cx: &mut Context<'_>) -> Poll<Result<(Self::Io, Self::Addr)>>;

/// Accepts a new incoming connection from this listener.
fn accept(&mut self) -> ListenerAcceptFut<'_, Self>
where
Self: Sized,
{
ListenerAcceptFut::new(self)
}

/// Returns the local address that this listener is bound to.
fn local_addr(&self) -> Result<Self::Addr>;
}

impl Listener for tokio::net::TcpListener {
type Io = tokio::net::TcpStream;
type Addr = std::net::SocketAddr;

fn poll_accept(&mut self, cx: &mut Context<'_>) -> Poll<Result<(Self::Io, Self::Addr)>> {
Self::poll_accept(self, cx)
}

fn local_addr(&self) -> Result<Self::Addr> {
self.local_addr().map(Into::into)
}
}

/// Future for accepting a new connection from a listener.
#[derive(Debug)]
pub struct ListenerAcceptFut<'a, L> {
cecton marked this conversation as resolved.
Show resolved Hide resolved
listener: &'a mut L,
}

impl<'a, L> ListenerAcceptFut<'a, L>
where
L: Listener,
{
fn new(listener: &'a mut L) -> Self {
Self { listener }
}
}

impl<'a, L> Future for ListenerAcceptFut<'a, L>
where
L: Listener,
{
type Output = Result<(L::Io, L::Addr)>;

fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
self.listener.poll_accept(cx)
}
}

impl<L, R> Listener for Either<L, R>
where
L: Listener,
R: Listener,
{
type Io = Either<<L as Listener>::Io, <R as Listener>::Io>;
type Addr = Either<<L as Listener>::Addr, <R as Listener>::Addr>;

fn poll_accept(&mut self, cx: &mut Context<'_>) -> Poll<Result<(Self::Io, Self::Addr)>> {
match self {
Either::Left(listener) => listener
.poll_accept(cx)
.map(|res| res.map(|(stream, addr)| (Either::Left(stream), Either::Left(addr)))),
Either::Right(listener) => listener
.poll_accept(cx)
.map(|res| res.map(|(stream, addr)| (Either::Right(stream), Either::Right(addr)))),
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm. It's unfortunate that the types here allow it return stuff like (Left(io), Right(addr)) even though it wont in practice. Ideas?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wow I'm so blind I didn't even notice.

Yes you're right in practice this will never happen. If someones use match on the Result, they will need to make unreachable branches just for that. It's stupid.

.... 🤔 I will think about it... but I don't see a solution right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably making yet another Either specialized for Listener could solve the issue.

I noticed the Either type is something not universal anyway: tokio-util has one but tower also has its own: https://docs.rs/tower/latest/tower/util/enum.Either.html

I assume we could have a "EitherListener" with a poll_accept method that would return Result<Either<(IoLeft, AddrLeft), (IoRight, AddrRight)>>. The enum itself would look like this I guess:

pub enum EitherListener<L: Listener, R: Listener> {
    Left(L),
    Right(R),
}

I don't think the user absolutely "needs" to use tokio-util's Either, it could definitely be something more specialized. wdyt?

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 not convinced that adding an EitherListener enum changes anything here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

EitherListener would not implement Listener, it would have its own method with their own signatures

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another option is to change the trait so that Addr and Io are merged into a single type which each listener will define as a pair.

That would work too but not super elegant I guess...

You could just put the method on the Either we already have.

Good point

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I replaced the Listener impl on Either by custom methods here: b55b62c3

Unfortunately the accept Future struct is slightly different so I had to duplicate some code.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just make it an async fn and drop the poll function? We don't need these tricks when it's not a trait method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤯

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 316f015


fn local_addr(&self) -> Result<Self::Addr> {
match self {
Either::Left(listener) => {
let addr = listener.local_addr()?;
Ok(Either::Left(addr))
}
Either::Right(listener) => {
let addr = listener.local_addr()?;
Ok(Either::Right(addr))
}
}
}
}
18 changes: 18 additions & 0 deletions tokio-util/src/net/unix/mod.rs
@@ -0,0 +1,18 @@
//! Unix domain socket helpers.

use super::Listener;
use std::io::Result;
use std::task::{Context, Poll};

impl Listener for tokio::net::UnixListener {
type Io = tokio::net::UnixStream;
type Addr = tokio::net::unix::SocketAddr;

fn poll_accept(&mut self, cx: &mut Context<'_>) -> Poll<Result<(Self::Io, Self::Addr)>> {
Self::poll_accept(self, cx)
}

fn local_addr(&self) -> Result<Self::Addr> {
self.local_addr().map(Into::into)
}
}