mirror of https://github.com/facebook/rocksdb.git
FaultInjectionTestFS::InjectThreadSpecificReadError() should not corrupt mmaped bytes (#8952)
Summary: Right now FaultInjectionTestFS::InjectThreadSpecificReadError() might try to corrupt return bytes, but these bytes might be from mmapped files, which would cause segfault. Instead FaultInjectionTestFS::InjectThreadSpecificReadError() should never corrupt data unless it is in caller's buffer. Pull Request resolved: https://github.com/facebook/rocksdb/pull/8952 Test Plan: See db_stress still runs and make sure in a test run failurs are still injected in non-mmap cases. Reviewed By: ajkr, ltamasi Differential Revision: D31147318 fbshipit-source-id: 9484a64ff2aaa36685557203f449286e694e65f9
This commit is contained in:
parent
b25f2afeff
commit
7c6a7e8fa8
|
@ -768,7 +768,7 @@ void FaultInjectionTestFS::UntrackFile(const std::string& f) {
|
|||
}
|
||||
|
||||
IOStatus FaultInjectionTestFS::InjectThreadSpecificReadError(
|
||||
ErrorOperation op, Slice* result, bool direct_io, char* /*scratch*/,
|
||||
ErrorOperation op, Slice* result, bool direct_io, char* scratch,
|
||||
bool need_count_increase, bool* fault_injected) {
|
||||
bool dummy_bool;
|
||||
bool& ret_fault_injected = fault_injected ? *fault_injected : dummy_bool;
|
||||
|
@ -803,11 +803,15 @@ IOStatus FaultInjectionTestFS::InjectThreadSpecificReadError(
|
|||
*result = Slice();
|
||||
ctx->message += "inject empty result; ";
|
||||
ret_fault_injected = true;
|
||||
} else if (!direct_io && Random::GetTLSInstance()->OneIn(7)) {
|
||||
} else if (!direct_io && Random::GetTLSInstance()->OneIn(7) &&
|
||||
scratch != nullptr && result->data() == scratch) {
|
||||
assert(result);
|
||||
// With direct I/O, many extra bytes might be read so corrupting
|
||||
// one byte might not cause checksum mismatch. Skip checksum
|
||||
// corruption injection.
|
||||
// We only corrupt data if the result is filled to `scratch`. For other
|
||||
// cases, the data might not be able to be modified (e.g mmaped files)
|
||||
// or has unintended side effects.
|
||||
// For a small chance, set the failure to status but corrupt the
|
||||
// result in a way that checksum checking is supposed to fail.
|
||||
// Corrupt the last byte, which is supposed to be a checksum byte
|
||||
|
|
Loading…
Reference in New Issue