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

Unexpectd behaviour of tokio::io::read when using tokio::io::read_until #1662

Closed
gliderkite opened this issue Oct 15, 2019 · 1 comment
Closed

Comments

@gliderkite
Copy link
Contributor

Version

rustc 1.38.0 (625451e37 2019-09-23)
└── tokio v0.1.22
    ├── tokio-codec v0.1.1
    │   └── tokio-io v0.1.12
    ├── tokio-current-thread v0.1.6
    │   └── tokio-executor v0.1.8
    ├── tokio-executor v0.1.8 (*)
    ├── tokio-fs v0.1.6
    │   ├── tokio-io v0.1.12 (*)
    │   └── tokio-threadpool v0.1.16
    │       └── tokio-executor v0.1.8 (*)
    │   └── tokio-io v0.1.12 (*)
    ├── tokio-io v0.1.12 (*)
    ├── tokio-reactor v0.1.10
    │   ├── tokio-executor v0.1.8 (*)
    │   ├── tokio-io v0.1.12 (*)
    │   └── tokio-sync v0.1.7
    ├── tokio-sync v0.1.7 (*)
    ├── tokio-tcp v0.1.3
    │   ├── tokio-io v0.1.12 (*)
    │   └── tokio-reactor v0.1.10 (*)
    ├── tokio-threadpool v0.1.16 (*)
    ├── tokio-timer v0.2.11
    │   └── tokio-executor v0.1.8 (*)
    ├── tokio-udp v0.1.5
    │   ├── tokio-codec v0.1.1 (*)
    │   ├── tokio-io v0.1.12 (*)
    │   └── tokio-reactor v0.1.10 (*)
    └── tokio-uds v0.2.5
        ├── tokio-codec v0.1.1 (*)
        ├── tokio-io v0.1.12 (*)
        └── tokio-reactor v0.1.10 (*)

Platform

Linux xps-15 5.0.0-31-generic #33-Ubuntu SMP Mon Sep 30 18:51:59 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

Description

I have a TCP server that is supposed to send the content of a file to multiple TCP clients following multiple concurrent requests.

Sometimes, using always the same inputs, the data received by the client is less than expected, therefore the transfer fails.

A full minimal working example can be found here, as well as all the steps necessary to reproduce the issue.

The error is not raised 100% of the times (that is, some of the client requests still succeed), but with the 100 tries defined in tcp_client.rs it was always reproducible for at least one of them.

The sequence of data transferred between client and server is composed by:

  1. the client send a request
  2. the server read the request and send a response
  3. the client read the response
  4. the server send the file data
  5. the client read the file data

This issue is only reproducible only if steps 1, 2 and 3 are involved, otherwise it works as expected.

The error is raised when this tokio::io::read (used to read the file content) returns 0, as if the server closed the connection, even is the server is actually up and running, and all the data has been sent (there is an assertion after tokio::io::copy and I checked the TCP packets using a packet sniffer). On a side note, in all my runs the amount of data read before the error was always > 95% than the one expected.

Most importantly the common.rs module defines 2 different read_* functions:

The logic of the 2 is the same, they need to read the request/response (and both client and server can be updated to use one or the other). What is surprising is that the bug presents itself only when tokio::io::read_until is used, while tokio::io::read_exact works as expected.

Unless, I misused tokio::io::read_until or there is a bug in my implementation, I expected both versions to work without any issue. What I am seeing instead is this panic being raised because some clients cannot read all the data sent by the server.

@gliderkite
Copy link
Contributor Author

gliderkite commented Oct 17, 2019

This is due to the creation of an instance of std::io::BufReader on the TcpStream: https://stackoverflow.com/questions/58439125/can-stdiobufreader-on-a-tcpstream-lead-to-data-loss.

JohnTitor added a commit to JohnTitor/rust that referenced this issue Nov 8, 2019
…=KodrAus

Enhance the documentation of BufReader for potential data loss

This is (IMO) and enhancement of the `std::io::BufReader` documentation, that aims to highlight how relatively easy is to end up with data loss when improperly using an instance of this class.

This is following the issue I had figuring out why my application was loosing data, because I focused my attention on the word *multiple instances* of `BufReader` in its `struct` documentation, even if I ever only had one instance.

Link to the issue: tokio-rs/tokio#1662
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

1 participant