crash in capacity overflow on SequentialReader in vec alloc

[harryhaaren]

Hi All,

I'm seeing a crashing issue after a few days of serving pages successfully. I'm not sure what the incoming request is (if there is one specific one is also unkown..) it seems like an out-of-memory "OOM" type issue, but the capacity-overflow error, which also seems to occur when .collect() is called on an infinite iterator (user-lang bug report & related discussion).

A similar issue seems to have occurred in rust-analyzer at some point but I'm not sure if/how that solution applies to the tiny-http codebase.

I've added a std::env::set_var() call to set the RUST_BACKTRACE=full for the program, which gives the below stack-trace for indicating where things are going wrong (as suggested in the discussion here)

Thoughts or input appreciated; this is for a toy website but I'd like to understand/fix it.

/// normal behaviour, serving pages as expected, then:

thread '<unnamed>' panicked at library/alloc/src/raw_vec.rs:545:5:
capacity overflow
stack backtrace:
   0:     0x56070a094dfc - std::backtrace_rs::backtrace::libunwind::trace::heeb1a1d1e2dbc154 at /rustc/de686cbc65478db53e3d51c52497685e852cc092/library/std/src/../../backtrace/src/backtrace/libunwind.rs:104:5
   1:     0x56070a094dfc - std::backtrace_rs::backtrace::trace_unsynchronized::h59b496b09972492a at /rustc/de686cbc65478db53e3d51c52497685e852cc092/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
... <snip bunch of rust-panic backtrace code> ...
  15:     0x560709ebc905 - core::panicking::panic_fmt::h1f0a3a584d873149 at /rustc/de686cbc65478db53e3d51c52497685e852cc092/library/core/src/panicking.rs:72:14
  16:     0x56070a0b49cf - alloc::raw_vec::capacity_overflow::h4830d61c7a2080df at /rustc/de686cbc65478db53e3d51c52497685e852cc092/library/alloc/src/raw_vec.rs:545:5
  17:     0x560709fdb746 - <tiny_http::util::equal_reader::EqualReader<R> as core::ops::drop::Drop>::drop::hcef8d364da48367b
  18:     0x560709fb843c - core::ptr::drop_in_place<tiny_http::util::equal_reader::EqualReader<tiny_http::util::sequential::SequentialReader<std::io::buffered::bufreader::BufReader<tiny_http::util::refined_tcp_stream::RefinedTcpStream>>>>::h6a8ca82231c133d5
  19:     0x560709ecb15b - core::ptr::drop_in_place<tiny_http::request::Request>::h9d7ea4fa29835dc0
  20:     0x560709ecdd7d - my_webserver::serve_https::h853dce33341b3390

[harryhaaren]

Open questions

• What does the EqualReader do?
• What does the SequentialReader do?
• Why is there an alloc::raw_vec occurring during Drop of the above instances?

I'm not familiar with the code (yet) so have these investigations as next steps... if folks know what's going on, or pointers/suggestions, do comment here :)

[kolbma]

Do you know anything new?

[kolbma]

@harryhaaren To answer your questions...

EqualReader would be better named like LimitReader.
It reads up to the specified number of bytes from another reader and not more. But is flexible if there are less bytes.

In the drop of EqualReader, the provided other reader is for sure read, so that the other reader is like seeked after the specified number of bytes. If the EqualReader has not been read before.

This reading is done with an allocated Vec buffer.

The SequentialReader provides the possibility that the server can handle multiple concurrent data streams by a client.

I guess because the other reader could be used later again, the EqualReader needs to do the seek. If both readers would always drop, it would not make any sense to do this stuff in drop and could be optimized.

I've provided a fix above, you could try to test it under your conditions.
Without this fix, the drop creates a Vec with the size of provided Content-Length header and so a OOM is possible with just using a wrong header with a very big number (u64::MAX).

You might also be interested in #232 to stop OOM possibility in headers.

Next to this I've not found any cause why there should be a capacity overflow.

[kolbma]

Replaced the Vec with a sliced stack array for performance...

test stack_buf_128     ... bench:   2,520,140 ns/iter (+/- 175,758)
test stack_buf_192     ... bench:   2,429,559 ns/iter (+/- 145,934)
test stack_buf_256     ... bench:   2,368,500 ns/iter (+/- 77,421)
test stack_buf_384     ... bench:   5,179,250 ns/iter (+/- 171,965)
test stack_buf_512     ... bench:   5,457,417 ns/iter (+/- 219,768)
test vec_128           ... bench:  10,220,704 ns/iter (+/- 317,712)
test vec_192           ... bench:  10,855,938 ns/iter (+/- 407,135)
test vec_256           ... bench:  10,921,623 ns/iter (+/- 782,854)
test vec_384           ... bench:  10,206,575 ns/iter (+/- 363,215)
test vec_512           ... bench:  10,810,583 ns/iter (+/- 255,277)
test vec_stack_buf_128 ... bench:   6,190,262 ns/iter (+/- 172,463)
test vec_stack_buf_192 ... bench:   7,972,014 ns/iter (+/- 279,520)
test vec_stack_buf_256 ... bench:   6,722,798 ns/iter (+/- 308,850)
test vec_stack_buf_384 ... bench:   6,945,378 ns/iter (+/- 289,744)
test vec_stack_buf_512 ... bench:   6,915,483 ns/iter (+/- 301,180)

kolbma@2af596e

    fn drop(&mut self) {
        if self.size == 0 {
            return;
        }

        let mut buf = &mut [0u8; 256][..];
        if self.size < 256 {
            buf = &mut buf[..self.size];
        }

[harryhaaren]

@harryhaaren To answer your questions...

Thanks!

I've provided a fix above, you could try to test it under your conditions. Without this fix, the drop creates a Vec with the size of provided Content-Length header and so a OOM is possible with just using a wrong header with a very big number (u64::MAX).

You might also be interested in #232 to stop OOM possibility in headers.

Next to this I've not found any cause why there should be a capacity overflow.

Aha; I've ported the site to Hyper, using Tokio etc now. Bigger picture, the goal of the codebase is to learn "hands on" rust networking in the real-world, so porting from threaded -> async was always a learning-goal.

Thanks for the responses; and building the library, it was a pleasure to use and get "raw" HTTP with Rust! Regards, -Harry