Scarfski/filament
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

GL backend: fix race condition when asserting a handle exists

Browse Source
This commit is contained in:
Mathias Agopian
2021-02-22 23:24:31 -08:00
committed by Mathias Agopian
parent 3a15756c78
commit bc6acd5c5a
3 changed files with 37 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
filament/backend/src/opengl/OpenGLDriver.cpp
View File

@@ -361,9 +361,7 @@ template<typename D, typename ... ARGS>
backend::Handle<D> OpenGLDriver::initHandle(ARGS&& ... args) noexcept {
static_assert(sizeof(D) <= 208, "Handle<> too large");
backend::Handle<D> h{ allocateHandle(sizeof(D)) };
#ifndef NDEBUG
mHandleSet.insert(h.getId());
#endif
registerHandleId(h.getId());
D* addr = handle_cast<D *>(h);
@@ -396,9 +394,6 @@ template <typename B, typename D, typename>
void OpenGLDriver::destruct(Handle<B>& handle, D const* p) noexcept {
// allow to destroy the nullptr, similarly to operator delete
if (p) {
#ifndef NDEBUG
assert_invariant(mHandleSet.find(handle.getId()) != mHandleSet.end());
#endif
#if !defined(NDEBUG) && UTILS_HAS_RTTI
if (UTILS_UNLIKELY(p->typeId != typeid(D).name())) {
slog.e << "Destroying handle " << handle.getId() << ", type " << typeid(D).name()
@@ -409,9 +404,7 @@ void OpenGLDriver::destruct(Handle<B>& handle, D const* p) noexcept {
#endif
p->~D();
mHandleArena.free(const_cast<D*>(p), sizeof(D));
#ifndef NDEBUG
mHandleSet.erase(handle.getId());
#endif
unregisterHandleId(handle.getId());
}
}

27
filament/backend/src/opengl/OpenGLDriver.h
View File

@@ -254,11 +254,34 @@ private:
utils::LockingPolicy::SpinLock,
utils::TrackingPolicy::Debug>;
utils::SpinLock mHandleSetLock;
tsl::robin_set<backend::HandleBase::HandleId> mHandleSet;
void registerHandleId(backend::HandleBase::HandleId id) noexcept {
mHandleSetLock.lock();
auto result = mHandleSet.insert(id);
assert_invariant(result.second);
mHandleSetLock.unlock();
}
void unregisterHandleId(backend::HandleBase::HandleId id) noexcept {
mHandleSetLock.lock();
assert_invariant(mHandleSet.find(id) != mHandleSet.cend() );
mHandleSet.erase(id);
mHandleSetLock.unlock();
}
void assertHandleId(backend::HandleBase::HandleId id) noexcept {
mHandleSetLock.lock();
assert_invariant(mHandleSet.find(id) != mHandleSet.cend() );
mHandleSetLock.unlock();
}
#else
using HandleArena = utils::Arena<HandleAllocator,
utils::LockingPolicy::SpinLock>;
inline void registerHandleId(backend::HandleBase::HandleId id) noexcept {}
inline void unregisterHandleId(backend::HandleBase::HandleId id) noexcept {}
inline void assertHandleId(backend::HandleBase::HandleId id) noexcept {}
#endif
HandleArena mHandleArena;
@@ -291,9 +314,7 @@ private:
handle_cast(backend::Handle<B>& handle) noexcept {
assert_invariant(handle);
if (!handle) return nullptr; // better to get a NPE than random behavior/corruption
#ifndef NDEBUG
assert_invariant(mHandleSet.find(handle.getId()) != mHandleSet.end());
#endif
assertHandleId(handle.getId());
char* const base = (char *)mHandleArena.getArea().begin();
size_t offset = handle.getId() << HandleAllocator::MIN_ALIGNMENT_SHIFT;
// assert that this handle is even a valid one

13
libs/utils/src/debug.cpp
View File

@@ -20,9 +20,18 @@
namespace utils {
void panic(const char *func, const char * file, int line, const char *assertion) noexcept {
PANIC_LOG("%s:%d: failed assertion `%s'\n", file, line, assertion);
// we use a non-inlined, not marked as "no return" function for aborting so that we can set
// a breakpoint on the call to abort() in panic() below and skip over it in the debugger if
// needed.
UTILS_NOINLINE
void abort() noexcept {
std::abort();
}
void panic(const char *func, const char * file, int line, const char *assertion) noexcept {
PANIC_LOG("%s:%d: failed assertion `%s'\n", file, line, assertion);
abort(); // set a breakpoint here
return; // this line is needed to be able to move the cursor here in the debugger
}
} // namespace filament