Locally copy queue to avoid holding lock while Actions execute. #14
Conversation
| lock (_executionQueue) | ||
| { | ||
| queueCopy = new Queue<Action>(_executionQueue); | ||
| _executionQueue.Clear(); |
There was a problem hiding this comment.
What is the reason for clearing the queue here? Because it's being moved over to an entirely new queue? I get the idea but it seems a lot can go wrong here.
There was a problem hiding this comment.
Can you say more about what could go wrong?
The copy-and-clear step is effectively the same as the previous while-not-empty-dequeue; we're just moving the Invoke step outside of the lock. The invariant from the original implementation should still be the same - that each action in _executionQueue when Update is called gets executed on the main thread and that those actions are no longer in the queue.
This just means background threads only wait for a queue to copy instead of arbitrarily expensive actions to execute. (Admittedly, if the actions are that expensive, they likely shouldn't be executing on the main thread, but that's a different issue).
Minor optimization - don't hold lock while executing queued actions.