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

Impossible to implement async IntoWs? #198

Open
bspeice opened this issue Nov 3, 2018 · 2 comments
Open

Impossible to implement async IntoWs? #198

bspeice opened this issue Nov 3, 2018 · 2 comments

Comments

@bspeice
Copy link

bspeice commented Nov 3, 2018

My goal is to build an object-pool Codec implementation to avoid allocations for all the frames/messages that come through a websocket. However, despite server/mod.rs mentioning the IntoWs trait, it appears impossible to implement in a separate crate because Upgrade in upgrade/async.rs is a struct that I can't patch the implementation for. Thus, I'd be able to adapt the underlying stream (so having new exotic things beyond just what tokio provides), but the protocol details are hidden from other crates.

Am I understanding this correctly? And is that all the more reason for #180 or extracting WsUpgrade into a trait?

@vi
Copy link
Member

vi commented Nov 3, 2018

Why do you need your own implementation of IntoWs?

There is a blanket implementation provided for all AsyncRead + AsyncWrite + 'static.

Non-allocating buffers look a non-small change in rust-websocket architecture and is probably not worth pursuing now, given next-gen rust-websocket may be on the way.

You can also experiment with patching rust-websocket code using replace section in Cargo.toml.

@bspeice
Copy link
Author

bspeice commented Nov 3, 2018

Non-allocating buffers isn't the issue, the IntoWs implementation is because that controls the Upgrade object being returned which ultimately determines the MessageProtocol used. I want to swap out MessageProtocol but in order to use the rest of the infrastructure I have to provide IntoWs.

EDIT: I should also mention that replacement via Cargo doesn't make much sense as I'd only need to edit one or two files, forking makes more sense at that point.

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

No branches or pull requests

2 participants