[Backport release-3_10] Fix Task manager waitforfinished

[rldhont]

Description

Manually backport #32838 needed for #36624

This PR fixes Task::waitForFinished methods. The method doesn't wait the task to be finished if the task has not yet being started (because the task wait to start if there is no thread available for instance).

It could lead to crashes (If you access some shared memory data believing the task has finished to run)

[troopa81]

Just for the record, there is other race conditions issues fixed in here #35200. But the one you backport could be enough to fix your issue.

I decided on purpose to not backport those because it was a lot of critical modifications in LTR. So far, no one has ever complained in master and 3.12 but I'm still unsure about backporting it. @nyalldawson any opinion on this

[rldhont]

Thanks @troopa81 for your comments.

@elpaso can I request your point of you about this backport needed to fix an issue in QGIS Server #36624 ?

[nyalldawson]

I'm -0 to backporting these, simply because the consequences of a regression are extreme. (On the other hand, I haven't seen any issues raised since the fixes landed in 3.12/master)

If we do decide to backport them, then you'll need to backport ALL the task manager fixes, as one of the follow up commits may also be required.

[rldhont]

Thanks @nyalldawson for your feedback.

If I cannot merge this backport, I will re-introduced and fix an event loop to wait for task starting in QgsAbstractContentCache.

[elpaso]

@rldhont I trust @nyalldawson 's opinion on this one, I've been bitten by the event loop recently and the side effects of an event loop are really dangerous but on the other hand we've lived with that for quite a long time.

The choice between backporting all the qgstask patches or reintroduce the event loop is ultimately up to you, I'm sorry but I can't tell which one is the less risky here.

[rldhont]

If I can't backport only this bugfix, I don't want to take the risk to backport all the QgsTask if @nyalldawson think it's risky.

I have reintroduced the event loop in #36624. If I can merge #36624 with the event loop I will close this one.

[nyalldawson]

I'm -1 to introducing an event loop here. That's an extremely dangerous change, totally unsuitable for LTR.

[rldhont]

@nyalldawson I have not introduced a new event loop in LTR. I have just fix an already existing event loop in LTR.

[rldhont]

If no reject #36651, I will merge it and close this one

[rldhont]

#36624 has been merged, so I close this one