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

TCP, FramedTCP message send is not truly async #177

Open
PPakalns opened this issue Jul 24, 2024 · 3 comments
Open

TCP, FramedTCP message send is not truly async #177

PPakalns opened this issue Jul 24, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@PPakalns
Copy link

PPakalns commented Jul 24, 2024

As seen in TODO comment and behaviour on io::ErrorKind::WouldBlock.

In server case this blocks communication with other clients.

I stumbled upon a situation when a faulty virtual network tunnel doesn't process sent data and whole program gets stuck in this loop and has 100% CPU utilization. It causes issues and delays in other situations too with large messages.

https://github.com/lemunozm/message-io/blob/master/src/adapters/tcp.rs#L187

    fn send(&self, data: &[u8]) -> SendStatus {
        // TODO: The current implementation implies an active waiting,
        // improve it using POLLIN instead to avoid active waiting.
        // Note: Despite the fear that an active waiting could generate,
        // this only occurs in the case when the receiver is full because reads slower that it sends.
        let mut total_bytes_sent = 0;
        loop {
            let mut stream = &self.stream;
            match stream.write(&data[total_bytes_sent..]) {
                Ok(bytes_sent) => {
                    total_bytes_sent += bytes_sent;
                    if total_bytes_sent == data.len() {
                        break SendStatus::Sent
                    }
                }
                Err(ref err) if err.kind() == io::ErrorKind::WouldBlock => continue,

                // Others errors are considered fatal for the connection.
                // a Event::Disconnection will be generated later.
                Err(err) => {
                    log::error!("TCP receive error: {}", err);
                    break SendStatus::ResourceNotFound // should not happen
                }
            }
        }
    }
@lemunozm lemunozm added the bug Something isn't working label Jul 24, 2024
@lemunozm
Copy link
Owner

lemunozm commented Jul 24, 2024

Hi, thanks for bringing this issue to light!

Yes, if the way of unlocking the receiver queue is in the same thread as this sender, then it will cause 100% of the CPU without the possibility of unlocking it. That should definitely be fixed using a POLLIN

@PPakalns
Copy link
Author

PPakalns commented Jul 24, 2024

Workaround implementation of AsyncFrameAdapter: master...PPakalns:message-io:async_framed_tcp

Of course, this adds additional cost of storing queue of buffers to be sent over the network.

@lemunozm
Copy link
Owner

Great! Feel free to open a PR with this new adapter. It can be interesting for users using tcp as you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants