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

feat: Add map_same_handle function to Async #108

Closed
wants to merge 1 commit into from

Conversation

notgull
Copy link
Member

@notgull notgull commented Jan 17, 2023

Recently, I've found myself using async-io in the use case (specifically, psychon/x11rb#790) where a TcpStream or a UnixStream can be used in the same spot, such that:

enum DefaultStream {
    Tcp(TcpStream),
    Unix(UnixStream),
}

let conn: Async<DefaultStream> = /* ... */;

The connection logic looks something like this:

async fn connect() -> Result<Async<DefaultStream>> {
    if do_tcp {
        let tcp = Async::<TcpStream>::connect(/* ... */).await?;
        Ok(Async::new(DefaultStream::Tcp(tcp.into_inner()?))?)
    } else {
        let unix = Async::<UnixStream>::connect(/* ... */).await?;
        Ok(Async::new(DefaultStream::Unix(unix.into_inner()?))?)
    }
}

Deregistering and then registering the connection into the reactor for this kind of simple map feels wasteful. This function would add a way to preform this kind of mapping without needing to deregister/register in the reactor.

@taiki-e
Copy link
Collaborator

taiki-e commented Jan 20, 2023

Thanks! I think it seems to make sense to have such a feature, but I'm not sure what a good name would be.

@notgull
Copy link
Member Author

notgull commented Jan 20, 2023

I've realized that this may not be the best idea to add. If we move forward with rustix and I/O safety in polling, it may become a safety requirement to ensure that the I/O source is removed from the poller before it is dropped (see this for a better explanation). If we move in that direction, then this function would allow us to invoke unsound behavior in the reactor.

What are your thoughts?

@taiki-e
Copy link
Collaborator

taiki-e commented Jan 21, 2023

Ah, good point, this indeed may not interact well with io safety.

A full switch to IO safety (i.e., replacing AsRaw* bounds with As* bounds) is a breaking change, and since async-io is a public dependency of smol it probably won't happen in the immediate future, but adding a feature that is likely to be removed without replacement in the next major version is not very nice.

(Also, in such cases, you might be able to use Either type of futures/futures-util.)

@notgull notgull closed this Jan 21, 2023
@notgull notgull deleted the notgull/map_same_handle branch January 26, 2023 02:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants