Add new test exposing mysterious bug #8
Conversation
| &mut write::Bz3Encoder::new(&mut output, 100 * KB).unwrap(), | ||
| ) | ||
| .unwrap(); | ||
|
|
||
| output | ||
| }; | ||
|
|
||
| let decompressed = { | ||
| let mut output = vec![]; | ||
| io::copy( | ||
| &mut read::Bz3Decoder::new(compressed.as_slice()).unwrap(), |
There was a problem hiding this comment.
To make sure that this test is coherent, you can apply these diffs:
- &mut write::Bz3Encoder::new(&mut output, 100 * KB).unwrap(),
+ &mut xz2::write::XzEncoder::new(&mut output, 6),and
- &mut read::Bz3Decoder::new(compressed.as_slice()).unwrap(),
+ &mut xz2::read::XzDecoder::new(compressed.as_slice()),Then run this:
cargo add --dev xz2
cargo test -- test_compressing_and_decompressing_small_inputAnd the test will pass, I am assuming xz2 is battle-tested and is behaving correctly right here.
marcospb19
changed the title
|
Ok merged. I'll take a look. |
|
Wait, you say the test in this PR will fail? But I tested it and no error encounters. ~/code/bzip3-rs master ⇡2 *1 ❯ cargo test test_compressing_and_decompressing_small_input 16:36:55
warning: /home/bczhc/code/bzip3-rs/Cargo.toml: version requirement `0.3.3+1.3.2` for dependency `libbzip3-sys` includes semver metadata which will be ignored, removing the metadata is recommended to avoid confusion
Finished test [unoptimized + debuginfo] target(s) in 0.18s
Running unittests src/lib.rs (target/debug/deps/bzip3-bfe851b9cfe33af3)
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
Running tests/test.rs (target/debug/deps/test-b5911d9001814aa1)
running 1 test
test test_compressing_and_decompressing_small_input ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 4 filtered out; finished in 0.00s
~/code/bzip3-rs master ⇡2 *1 ❯ cargo test -r test_compressing_and_decompressing_small_input 16:37:45
warning: /home/bczhc/code/bzip3-rs/Cargo.toml: version requirement `0.3.3+1.3.2` for dependency `libbzip3-sys` includes semver metadata which will be ignored, removing the metadata is recommended to avoid confusion
Finished release [optimized] target(s) in 0.18s
Running unittests src/lib.rs (target/release/deps/bzip3-a973389c0044f02a)
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
Running tests/test.rs (target/release/deps/test-7125935edaffd8d9)
running 1 test
test test_compressing_and_decompressing_small_input ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 4 filtered out; finished in 0.00s
|
|
Yeah, it was failing here locally and well as in your CI, right? |
|
Weird. Failed in the CI, but OK on my side. Maybe this issue should be dug in some more. |
|
I think it only has something to do with the Rust toolchain version... I may need to do some bisecting. With the latest stable toolchain it passes; with the latest nightly toolchain it fails. |
|
cargo-bisect result: OMG I will figure out the root cause later... |
|
Using my own fn my_io_copy<R: Read, W: Write>(mut reader: R, mut writer: W) -> io::Result<()> {
const BUF_SIZE: usize = 40960;
let mut buf = [0_u8; BUF_SIZE];
loop {
let size = reader.read(&mut buf)?;
if size == 0 {
return Ok(());
}
writer.write_all(&buf[..size])?;
}
} |
use std::io;
use std::io::Read;
fn main() {
let mut reader = DemoReader::new();
let mut vec = vec![];
let len = io::copy(&mut reader, &mut vec).unwrap();
println!("{} {:?}", len, vec);
}
/// Expect only one byte to read
struct DemoReader {
n: u32,
}
impl DemoReader {
fn new() -> DemoReader {
Self { n: 0 }
}
}
impl Read for DemoReader {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
assert!(buf.len() > 1);
println!("read call: {}, requested size: {}", self.n, buf.len());
match self.n {
0 => {
buf[0] = 1;
self.n += 1;
Ok(1)
}
1 => {
self.n += 1;
Ok(0)
}
2 => {
self.n += 1;
buf[0] = 2;
Ok(1)
}
3 => {
self.n += 1;
Ok(0)
}
_ => {
unreachable!()
}
}
}
}Here's a demo code snippet. Before rust-lang/rust#113493, the output is: read call: 0, requested size: 8192
read call: 1, requested size: 8192
1 [1]After that PR, it outputs: read call: 0, requested size: 8192
read call: 1, requested size: 8191
read call: 2, requested size: 8192
read call: 3, requested size: 16384
2 [1, 2]Seems I would need to take some time to check more and find a solution later. |
|
I made a hurry fix... Check it out. Sorry for my limited time and I will leave my laptop for ~1day from now on. |
|
Thanks for the help, I'm still getting other errors and trying to solve them. I'll let you know if I succeed. |
|
Hi, the change in behavior was unintended. rust-lang/rust#117576 should fix it. |
|
Oh i see lol. Thanks @the8472! |
…lacrum Fix excessive initialization and reads beyond EOF in `io::copy(_, Vec<u8>)` specialization fixes rust-lang#117545 and bczhc/bzip3-rs#8
Fix excessive initialization and reads beyond EOF in `io::copy(_, Vec<u8>)` specialization fixes #117545 and bczhc/bzip3-rs#8
Fix excessive initialization and reads beyond EOF in `io::copy(_, Vec<u8>)` specialization fixes #117545 and bczhc/bzip3-rs#8
Fix excessive initialization and reads beyond EOF in `io::copy(_, Vec<u8>)` specialization fixes #117545 and bczhc/bzip3-rs#8
Hi, it's me once again 👋.
Tests are failing in
ouchagain so I went investigating and minifying the error, until I landed at this minimal repro test that exposes some mysterious bug.Output looks duplicated and the other formats break when dealing with it.