fix processEvents() reentrancy bug in progress manager window handling

[kieranlblack]

My fix in #3029 was not fully correct, this PR should be a corrected version though.

@dae I was just thinking about this and I believe even though it seems to fix the being reported, the code I submitted still isn't fully free of race conditions due to the reentrancy of processEvents(). Consider the following scenario:

1. finish() is called with _levels = 1
2. _closeWin() is called and yields control via processEvents()
3. start() is called and increments _levels to 2
4. processEvents() returns
5. _win is cancelled and set to None
6. _levels is set to 0

So we end up in a state where _levels = 0 yet there is still a ProgressDialog active. It is a similar issue to as before just wandered into in a different way. Here we would never end up calling cancel() or isdeleted() on a ProgressDialog which is None but still have the issue of getting orphaned windows.

The reentrancy is somewhat tricky, I suppose we need to check the _levels "semaphore" on gaining control again. We can make the metaphorical bandaid slightly larger or remove it like you were saying. Some ideas:

• we move the window presented check into the start finish() and out of _closeWin() and do it first before updating any internal state, this way after finishing the check we are guaranteed that the value we read for _levels won't change
• we have _closeWin() check _levels on reentry, if it isn't 1 then we return False and can bail out in finish() after setting _levels appropriately
• we use taskman to wait until the window has been shown for a while and then on the done callback check all the other logic from the main thread, this removes the usage of processEvents()

Do you feel one of these is significantly better than the others?

I implemented the third option from the list of ideas as it removed the processEvents() call but am more than happy to pivot to something else entirely if you want! Just lmk.

I tested this by playing around with the app for a little bit and inserting artificial delays in an addon. The path where the window was never shown and where we need to delay for half a second both seem to be working. No infinite Processing... windows got stuck open.

[dae]

Removing processEvents() is probably the best path forward. Thank you for looking into this!

[dae]

Using sleep() in a background thread could be done more simply with self.single_shot()

[kieranlblack]

Oh nice that is perfect!

[kieranlblack]

@dae I just looked at this a little more and we can't actually use single_shot here since it wraps the callback and won't call it until _levels is 0 which is the whole point of our callback :/ I'll stick to the threadpool for now and add a comment about it.

[dae]

Sorry to lead you down the wrong path there - I'd forgotten about that check.

[dae]

Thanks for this Kieran!