Strengthen lifetime management guarantees of workers

[oktal3700]

Original report

swift crashes if I shutdown the application inbetween these steps

template <typename T, bool UseCompare>
CWorker *CListModelBase<T, UseCompare>::updateAsync(const ContainerType &container, bool sort)
{
    Q_UNUSED(sort);
    if (m_modelDestroyed) { return nullptr; }
    const auto sortColumn = this->getSortColumn();
    const auto sortOrder  = this->getSortOrder();
    CWorker *worker = CWorker::fromTask(this, "ModelSort", [this, container, sortColumn, sortOrder]()
    {
        return this->sortContainerByColumn(container, sortColumn, sortOrder);
    });
    worker->thenWithResult<ContainerType>(this, [this](const ContainerType & sortedContainer)
    {
        if (m_modelDestroyed) { return;  }
        this->update(sortedContainer, false);
    });
    worker->then(this, &CListModelBase::asyncUpdateFinished);
    return worker;

Discussion

The list model is destroyed while the worker is still sorting the container. Hence it crashes because it tries to sort a container that was destroyed.

The design of the workers was supposed to avoid this by using parent/child relation between owner (list model) and QThread, and making the QThread destructor wait for the worker to finish. But of course I overlooked that the derived-class owner is destroyed before its base class QObject destructor gets the chance to destroy the QThread child object.

The proximate cause seems to be here, in sortContainerByColumn:

// sort the values
const std::integral_constant<bool, UseCompare> marker {};
const auto p = [ = ](const ObjectType & a, const ObjectType & b) -> bool
{
    return Private::compareForModelSort<ObjectType>(a, b, order, propertyIndex, m_sortTieBreakers, marker);
//                                                                              ^~~~~~~~~~~~~~~~~
};

return container.sorted(p);

The lambda is implicitly capturing the this pointer, through which it accesses the m_sortTieBreakers member. It crashes when m_sortTieBreakers is destroyed.

As a workaround we now capture a copy of m_sortTieBreakers instead.

Returning to the broader, general issue, there are two different concerns:

1. Ensuring the worker does get deleted on every code path, so resources are not leaked.
2. Ensuring any data used by the worker does not get deleted before the worker.

(1) is ensured by the parent/child relation between owner and thread, which is good. But if the owner also owns some of the data used by the worker, then (2) becomes a problem of ensuring a particular order of destruction of owned objects, which is a hard problem.

I feel like the workaround above is actually suggestive of the best general, robust solution: workers need to always work on local copies of data, and never access anything outside themselves.