From ad3f18f031a0bd897f756d66486c2bc0216794a1 Mon Sep 17 00:00:00 2001 From: "Jeff Washington (jwash)" <75863576+jeffwashington@users.noreply.github.com> Date: Wed, 14 Jul 2021 19:04:55 -0500 Subject: [PATCH] fix race condition in bg file reader file error (#18682) --- runtime/src/shared_buffer_reader.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/runtime/src/shared_buffer_reader.rs b/runtime/src/shared_buffer_reader.rs index b52fab54fb..5bf7083661 100644 --- a/runtime/src/shared_buffer_reader.rs +++ b/runtime/src/shared_buffer_reader.rs @@ -186,6 +186,7 @@ impl SharedBufferBgReader { let mut max_bytes_read = 0; let mut wait_us = 0; let mut total_bytes = 0; + let mut error = SharedBufferReader::default_error(); loop { if self.stop.load(Ordering::Relaxed) { // unsure what error is most appropriate here. @@ -234,9 +235,9 @@ impl SharedBufferBgReader { bytes_read += size; // loop to read some more. Underlying reader does not usually read all we ask for. } - Err(err) => { + Err(mut err) => { error_received = true; - self.set_error(err); + std::mem::swap(&mut error, &mut err); break; } } @@ -259,6 +260,8 @@ impl SharedBufferBgReader { if error_received { // do not ask for more data from 'reader'. We got an error and saved all the data we got before the error. + // but, wait to set error until we have added our buffer to newly_read_data + self.set_error(error); break; } } @@ -692,10 +695,12 @@ pub mod tests { ); // #2 will read valid bytes first and succeed, then get error let mut data = vec![0; sent.len()]; - // this read should succeed because it was prior to error being received by bg reader + // this read should succeed because it is reading data prior to error being received by bg reader let expected_len = 1; for i in 0..sent.len() { - assert_eq!(reader2.read(&mut data[i..=i]).unwrap(), expected_len); + let len = reader2.read(&mut data[i..=i]); + assert!(len.is_ok(), "{:?}, progress: {}", len, i); + assert_eq!(len.unwrap(), expected_len, "progress: {}", i); } assert_eq!(sent, data); assert_eq!(