Fix undefined behavior from accessing field of uninitialized object

This commit is contained in:
Lukas Joswiak 2022-06-17 16:35:17 -07:00
parent 7bff4af14a
commit 4c2bb0b44e
2 changed files with 49 additions and 22 deletions

View File

@ -78,7 +78,8 @@ void ThreadSafeDatabase::setOption(FDBDatabaseOptions::Option option, Optional<S
db->checkDeferredError();
db->setOption(option, passValue.contents());
},
&db->deferredError);
db,
&DatabaseContext::deferredError);
}
ThreadFuture<int64_t> ThreadSafeDatabase::rebootWorker(const StringRef& address, bool check, int duration) {
@ -243,7 +244,7 @@ void ThreadSafeTransaction::cancel() {
void ThreadSafeTransaction::setVersion(Version v) {
ISingleThreadTransaction* tr = this->tr;
onMainThreadVoid([tr, v]() { tr->setVersion(v); }, &tr->deferredError);
onMainThreadVoid([tr, v]() { tr->setVersion(v); }, tr, &ISingleThreadTransaction::deferredError);
}
ThreadFuture<Version> ThreadSafeTransaction::getReadVersion() {
@ -402,12 +403,12 @@ void ThreadSafeTransaction::addReadConflictRange(const KeyRangeRef& keys) {
KeyRange r = keys;
ISingleThreadTransaction* tr = this->tr;
onMainThreadVoid([tr, r]() { tr->addReadConflictRange(r); }, &tr->deferredError);
onMainThreadVoid([tr, r]() { tr->addReadConflictRange(r); }, tr, &ISingleThreadTransaction::deferredError);
}
void ThreadSafeTransaction::makeSelfConflicting() {
ISingleThreadTransaction* tr = this->tr;
onMainThreadVoid([tr]() { tr->makeSelfConflicting(); }, &tr->deferredError);
onMainThreadVoid([tr]() { tr->makeSelfConflicting(); }, tr, &ISingleThreadTransaction::deferredError);
}
void ThreadSafeTransaction::atomicOp(const KeyRef& key, const ValueRef& value, uint32_t operationType) {
@ -415,7 +416,9 @@ void ThreadSafeTransaction::atomicOp(const KeyRef& key, const ValueRef& value, u
Value v = value;
ISingleThreadTransaction* tr = this->tr;
onMainThreadVoid([tr, k, v, operationType]() { tr->atomicOp(k, v, operationType); }, &tr->deferredError);
onMainThreadVoid([tr, k, v, operationType]() { tr->atomicOp(k, v, operationType); },
tr,
&ISingleThreadTransaction::deferredError);
}
void ThreadSafeTransaction::set(const KeyRef& key, const ValueRef& value) {
@ -423,14 +426,14 @@ void ThreadSafeTransaction::set(const KeyRef& key, const ValueRef& value) {
Value v = value;
ISingleThreadTransaction* tr = this->tr;
onMainThreadVoid([tr, k, v]() { tr->set(k, v); }, &tr->deferredError);
onMainThreadVoid([tr, k, v]() { tr->set(k, v); }, tr, &ISingleThreadTransaction::deferredError);
}
void ThreadSafeTransaction::clear(const KeyRangeRef& range) {
KeyRange r = range;
ISingleThreadTransaction* tr = this->tr;
onMainThreadVoid([tr, r]() { tr->clear(r); }, &tr->deferredError);
onMainThreadVoid([tr, r]() { tr->clear(r); }, tr, &ISingleThreadTransaction::deferredError);
}
void ThreadSafeTransaction::clear(const KeyRef& begin, const KeyRef& end) {
@ -445,14 +448,15 @@ void ThreadSafeTransaction::clear(const KeyRef& begin, const KeyRef& end) {
tr->clear(KeyRangeRef(b, e));
},
&tr->deferredError);
tr,
&ISingleThreadTransaction::deferredError);
}
void ThreadSafeTransaction::clear(const KeyRef& key) {
Key k = key;
ISingleThreadTransaction* tr = this->tr;
onMainThreadVoid([tr, k]() { tr->clear(k); }, &tr->deferredError);
onMainThreadVoid([tr, k]() { tr->clear(k); }, tr, &ISingleThreadTransaction::deferredError);
}
ThreadFuture<Void> ThreadSafeTransaction::watch(const KeyRef& key) {
@ -469,7 +473,7 @@ void ThreadSafeTransaction::addWriteConflictRange(const KeyRangeRef& keys) {
KeyRange r = keys;
ISingleThreadTransaction* tr = this->tr;
onMainThreadVoid([tr, r]() { tr->addWriteConflictRange(r); }, &tr->deferredError);
onMainThreadVoid([tr, r]() { tr->addWriteConflictRange(r); }, tr, &ISingleThreadTransaction::deferredError);
}
ThreadFuture<Void> ThreadSafeTransaction::commit() {
@ -481,16 +485,18 @@ ThreadFuture<Void> ThreadSafeTransaction::commit() {
}
Version ThreadSafeTransaction::getCommittedVersion() {
// This should be thread safe when called legally, but it is fragile
return tr->getCommittedVersion();
ISingleThreadTransaction* tr = this->tr;
return onMainThread([tr]() -> Future<Version> { return tr->getCommittedVersion(); }).get();
}
VersionVector ThreadSafeTransaction::getVersionVector() {
return tr->getVersionVector();
ISingleThreadTransaction* tr = this->tr;
return onMainThread([tr]() -> Future<VersionVector> { return tr->getVersionVector(); }).get();
}
SpanContext ThreadSafeTransaction::getSpanContext() {
return tr->getSpanContext();
ISingleThreadTransaction* tr = this->tr;
return onMainThread([tr]() -> Future<SpanContext> { return tr->getSpanContext(); }).get();
}
ThreadFuture<int64_t> ThreadSafeTransaction::getApproximateSize() {
@ -513,7 +519,9 @@ void ThreadSafeTransaction::setOption(FDBTransactionOptions::Option option, Opti
Standalone<Optional<StringRef>> passValue = value;
// ThreadSafeTransaction is not allowed to do anything with options except pass them through to RYW.
onMainThreadVoid([tr, option, passValue]() { tr->setOption(option, passValue.contents()); }, &tr->deferredError);
onMainThreadVoid([tr, option, passValue]() { tr->setOption(option, passValue.contents()); },
tr,
&ISingleThreadTransaction::deferredError);
}
ThreadFuture<Void> ThreadSafeTransaction::checkDeferredError() {

View File

@ -39,15 +39,24 @@
namespace internal_thread_helper {
ACTOR template <class F>
void doOnMainThreadVoid(Future<Void> signal, F f, Error* err) {
void doOnMainThreadVoid(Future<Void> signal, F f) {
wait(signal);
if (err && err->code() != invalid_error_code)
try {
f();
} catch (Error& e) {
}
}
ACTOR template <class F, class T>
void doOnMainThreadVoid(Future<Void> signal, F f, T* t, Error T::*member) {
wait(signal);
if (t && (t->*member).code() != invalid_error_code)
return;
try {
f();
} catch (Error& e) {
if (err)
*err = e;
if (t)
t->*member = e;
}
}
@ -63,11 +72,21 @@ void doOnMainThreadVoid(Future<Void> signal, F f, Error* err) {
// WARNING: The error returned in `err` can only be read on the FDB network thread because there is no way to
// order the write to `err` with actions on other threads.
//
// WARNING: The Error member of `T` is accepted as a pointer to a data member so the caller can avoid dereferencing
// `T` until it is initialized on the main thread.
//
// `onMainThreadVoid` is defined here because of the dependency in `ThreadSingleAssignmentVarBase`.
template <class F>
void onMainThreadVoid(F f, Error* err = nullptr, TaskPriority taskID = TaskPriority::DefaultOnMainThread) {
template <class F, class T>
void onMainThreadVoid(F f, T* t, Error T::*member, TaskPriority taskID = TaskPriority::DefaultOnMainThread) {
Promise<Void> signal;
internal_thread_helper::doOnMainThreadVoid(signal.getFuture(), f, err);
internal_thread_helper::doOnMainThreadVoid(signal.getFuture(), f, t, member);
g_network->onMainThread(std::move(signal), taskID);
}
template <class F>
void onMainThreadVoid(F f, std::nullptr_t = nullptr, TaskPriority taskID = TaskPriority::DefaultOnMainThread) {
Promise<Void> signal;
internal_thread_helper::doOnMainThreadVoid(signal.getFuture(), f);
g_network->onMainThread(std::move(signal), taskID);
}