Repository: google/filament
File: filament/src/details/Engine.cpp (C++)

Code before:
template<typename T, typename Lock>
UTILS_ALWAYS_INLINE
bool FEngine::terminateAndDestroyLocked(Lock& lock, const T* p, ResourceList<T>& list) {
    if (p == nullptr) return true;
    lock.lock();
    bool const success = list.remove(p);
    lock.unlock();

#if UTILS_HAS_RTTI
    auto typeName = CallStack::typeName<T>();
    const char * const typeNameCStr = typeName.c_str();
#else
    const char * const typeNameCStr = "<no-rtti>";
#endif

    if (ASSERT_PRECONDITION_NON_FATAL(success,
            "Object %s at %p doesn't exist (double free?)", typeNameCStr, p)) {
        const_cast<T*>(p)->terminate(*this);
        mHeapAllocator.destroy(const_cast<T*>(p));
    }
    return success;
}

template<typename T>
UTILS_NOINLINE
void FEngine::terminateAndDeferAsyncObjectDestruction(const T* p, ResourceList<T>& list) {
    bool const success = list.remove(p);

#if UTILS_HAS_RTTI
    auto typeName = CallStack::typeName<T>();
    const char * const typeNameCStr = typeName.c_str();
#else
    const char * const typeNameCStr = "<no-rtti>";
#endif

    if (ASSERT_PRECONDITION_NON_FATAL(success,
        "Object %s at %p doesn't exist (double free?)", typeNameCStr, p)) {
        // Terminate the backend resource immediately as they're unnecessary from this point.
        const_cast<T*>(p)->terminate(*this);

        // We defer the destruction of the frontend object until the creation process completes.
        // This ensures the object remains valid while the creation process still holds references
        // to it.
        mDeferredAsyncObjectDestruction.push_back([this, p=const_cast<T*>(p)]() {
            if (!p->isCreationComplete()) {
                return false;
            }
            mHeapAllocator.destroy(p);
            return true;
        });
    }

Review comment (suggestion):
instead of duplication most of the code here, wouldn't it make more sense to add a "bool async = false" parameter to `terminateAndDestroy`?  For most objects it would be `false` and no extra code would be added.

Code after:

        T* p = const_cast<T*>(ptr);

        if constexpr (HasIsCreationComplete<T>::value) {
            // The presence of 'isCreationComplete' in type T implies it supports asynchronous
            // creation. For these asynchronous objects, we can terminate the backend resources
            // immediately as they are no longer referenced. However, we defer the destruction of
            // the frontend object if it's still being loaded.
            // Reason: The creation process is still active and holds a reference to the frontend
            // object to set `mCreationComplete` to true (see FTexture::FTexture). Deleting it now
            // may cause a crash, so we wait until creation completes.

            // Terminate the backend resource immediately as they're unnecessary from this point.
            p->terminate(*this);

            if (p->isCreationComplete()) {
                // If creation is complete, we free the frontend object immediately. Note that in
                // regular (non-async) mode, the `isCreationComplete` method always return true.
                mHeapAllocator.destroy(p);
            } else {
                // We defer the destruction of the frontend object until the creation process
                // completes. This ensures the object remains valid while the creation process still
                // holds references to it.
                mDeferredAsyncObjectDestruction.push_back([this, p]() {
                    if (!p->isCreationComplete()) {
                        return false;
                    }
                    mHeapAllocator.destroy(p);
                    return true;
                });
            }
        } else {
            p->terminate(*this);
            mHeapAllocator.destroy(p);
        }
    }
    return success;
}

template<typename T, typename Lock>
UTILS_ALWAYS_INLINE
bool FEngine::terminateAndDestroyLocked(Lock& lock, const T* p, ResourceList<T>& list) {
    if (p == nullptr) return true;
    lock.lock();
    bool const success = list.remove(p);
    lock.unlock();

#if UTILS_HAS_RTTI
    auto typeName = CallStack::typeName<T>();
    const char * const typeNameCStr = typeName.c_str();
#else