Prevent multiple simultaneous polling loops

[matthewwalsh0]

Problem

The central mechanism of the PollingBlockTracker is the synchronize method which provides a loop to fetch the block number then wait for a given interval, all until the block tracker is no longer running meaning it no longer has any event listeners.

If all listeners are removed while the synchronize loop is waiting for the timeout, once the timeout finishes the loop will exit since isRunning will now be false.

If all listeners are removed but then some are added again, all while waiting for a timeout, this allows a new synchronise loop to be created since isRunning was false meaning _maybeStart > _start > _synchronize. Once the original timeout finishes however, it continues as normal since isRunning is true again meaning we now have two synchronize loops occurring simultaneously with different interval alignment meaning we ultimately poll more often than the desired polling interval. This problem can stack and create many simultaneous loops thereby greatly increasing the actual polling interval.

Test Steps

1. Close all extension tabs and popups.
2. Open the extension popup.
3. Close the extension popup.
4. Repeat steps 2 and 3 multiple times to increase the impact.
5. Open the extension and expand view into a new tab (to keep the incoming transaction listener present).
6. Note in the console that the block tracker makes requests as fast as every second.

Solution

There are multiple possible solutions but for the sake of the simplest final code, this PR replaces the synchronize loop with a setTimeout chain.

This allows duplicate "loops" to be easily avoided since we can simply clear any pending timeouts before creating a new one, plus do the same when all listeners are removed and the _end method is ultimately called.

In addition, we can simplify the _maybeStart, _maybeEnd, _start, and _end methods by removing the async keyword since no asynchronous logic is required. This introduces additional clarity in code path since we are invoking these methods from synchronous event listeners meaning it is ideal that we only require synchronous methods.

Note that updateAndQueue is an async method but we don't need to await this since we want to emit the _started event before the first iteration has been completed, and it includes its own error handling.

[mcmire]

Good work investigating this! This makes sense to me. I just had a couple of questions but they are non-blocking.

[legobeat]

@matthewwalsh0 I think we can ship this? :)

• 9.0.1 #236