Skip to content

add split traits to fill functionality gaps in downstream crates - #748

Open
celogic wants to merge 2 commits into
rust-embedded:masterfrom
celogic:features
Open

add split traits to fill functionality gaps in downstream crates#748
celogic wants to merge 2 commits into
rust-embedded:masterfrom
celogic:features

Conversation

@celogic

@celogic celogic commented Aug 19, 2026

Copy link
Copy Markdown

Adds a SplitRW trait for both async and non async versions.
It abstracts the capability, making them available for downstream libraries.

Example:
embedded_tls::TlsConnection builds on (abstract) <Socket: embedded_io_async::Read + embedded_io_async::Write>.
embedded_tls::TlsConnection impl the fn split, but it requires Socket: Clone.
this is not effective as Socket doesn't need to be completely cloned, which causes functionality gaps.
e.g. if i use embassy_net::tcp::TcpSocket as the underlaying Socket, I cant call TlsConnection::split,
because embassy_net::tcp::TcpSocket doesn't impl Clone, despite providing split itself

  • Why the name SplitRW? Split into Reader and Writer
  • Why not just Split? To not be confused with std::io::Split, which is an Iterator
  • SplitRW doesn't require Read + Write, because i don't think its necessary and leaves the implementor more flexible

@celogic
celogic requested a review from a team as a code owner August 19, 2026 10:18
@MabezDev

Copy link
Copy Markdown
Member

Splitting semantics get tricky, you haven't accounted for anything beyond just splitting (think: configuration that affects both sides etc) . I also don't believe this is a e-hal specific problem so I'm not sure it belongs here.

@Dirbaio

Dirbaio commented Aug 19, 2026

Copy link
Copy Markdown
Member

I got some concerns:

  • std::io doesn't have such a trait. Why? Has it ever been proposed, was it deemed not necessary for some reason? How do libs using std::io deal with this problem? Is embedded really different? IMO we shouldn't add something std::io doesn't have unless we have a good reason.
  • The signature isn't quite right. It takes borrowed self and returns owned halves. It should be either all borrowed or all owned. Or both. Which one we want is not obvious, depending on the case an impl may be able to do borrowed only, but for users owned is often more convenient.
trait BorrowedSplit {
    type Reader<'a>: Read where Self: 'a;
    type Writer<'a>: Write where Self: 'a;
    fn split_rw(&mut self) -> (Self::Reader<'_>, Self::Writer<'_>);
}
trait OwnedSplit {
    type Reader: Read;
    type Writer: Write;
    fn split_rw(self) -> (Self::Reader, Self::Writer);
}
  • Should it constrain the error types for the children to be the same? Maybe not since a lib that wants that can always do it with where clauses I think.

@celogic celogic closed this Aug 21, 2026
@Dirbaio

Dirbaio commented Aug 21, 2026

Copy link
Copy Markdown
Member

@celogic why close without any comment? My concerns don't necessarily mean we shouldn't add a Split trait, just that we should have a discussion beforehand of what'd be the best design for it.

It's a bit disrespectful to the maintainer to outright close with no comment after the maintainer has put in time to review.

@celogic

celogic commented Aug 22, 2026

Copy link
Copy Markdown
Author

@Dirbaio my apologies, my intention in closing this pr, was, to show that my design, as proposed, has issues and is not ready to be merged.
I tended to to reopen this pr with a better design, or open a pr on another crate.
You're right that I should have closed this pr with a comment – it's the first time I want to contribute to an open source project, so I still need to gain experience.
You have some valid concerns and I appreciate the time you put into this.
I'm currently investigating other designs with regard to your concerns.

p.s. should I reopen this pr?

@celogic

celogic commented Aug 24, 2026

Copy link
Copy Markdown
Author

@Dirbaio what about this design?

trait SplitRW /* borrowed */ {
    type Reader<'a>: Read where Self: 'a;
    type Writer<'a>: Write where Self: 'a;

    // p.s. i guess this fn is the same as yours, but explicit
    fn split_rw<'s>(&'s mut self) -> Result<(Self::Reader<'s>, Self::Writer<'s>), () /* optionally suitable error type or no result at all*/>;
}

// provide owned variant if it makes sense (not my primary interest)
trait IntoSplitRW /* owned */ {
    type Reader: Read;
    type Writer: Write;
    type Controller; // can be used to handle the underlaying socket e.g. set_timeout, close, set_keep_alive

    fn into_split_rw(self) -> Result<(Self::Reader, Self::Writer, Self::Controller), ()>;
}

// optionally the underlaying socket provides a fn to reassemble itself?
fn Socket::reassemble(Reader, Writer, Controller) -> Result<Self, ()>;

note that GAT (generic associated types) is a new feature in rust (1.65). maybe thats the reason why this trait (borrowed variant) is not provided in std::io

example:

use embassy_futures::join::join;

socket.open();

let (mut reader, mut writer) = socket.split().unwrap();

// or threads
join(
    async {
        reader.read(...);
        ...
    },
    async {
        writer.write();
       ...
    },
)
.await;

socket.close()

@celogic

celogic commented Aug 24, 2026

Copy link
Copy Markdown
Author

here is a quick example how I would implement it: https://github.com/celogic/split_demo
I had some trouble debugging lifetime issues, but now everything should work.

if somebody is curious:
p.s. its difficult to explain complex lifetimes for me
It turns out, TcpSocket::split is too restrictive with the lifetimes:
i guess

pub fn split(&mut self) -> (TcpReader<'_>, TcpWriter<'_>)

is the implicit form of

pub fn split<'s>(&'s mut self) -> (TcpReader<'s>, TcpWriter<'s>)

which implies, that TcpReader and TcpWriter take a reference to self, which they dont
more accurate would be

impl<'a> TcpSocket<'a> {
    pub fn split<'s>(&'s mut self) -> (TcpReader<'a>, TcpWriter<'a>)
}

if they would want TcpReader and TcpWriter to 'take' reference to TcpSocket (so that TcpSocket is not usable in the mean time), they should introduce a new lifetime parameter, with a phantom reference. e.g.:

pub struct TcpReader<'s, 'a> {
    io: TcpIo<'a>,
    _socket: PhantomData<&'s TcpSocket<'a>>,
}

impl<'a> TcpSocket<'a> {
    pub fn split<'s>(&'s mut self) -> (TcpReader<'s, 'a>, TcpWriter<'s, 'a>);
}

this mismatch causes lifetime issues in implementing SplitRW for TcpStream
i also tried implementing a custom split fn for TlsConnection where Socket: TcpStream, but this caused TlsConnection to be borrowed, even if TlsReader and TlsWriter got dropped (i guess because the compiler thinks that TlsConnection is borrowed for lifetime of TcpSocket's internal reference).

@celogic celogic reopened this Aug 26, 2026
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.

3 participants