Make sure the ticker thread can die gracefully

[djc]

Fixes #416. See also previous discussion in #417 and #421.

Thanks for working on this test! I reworked the test to avoid adding overhead outside running tests and reuse the once_cell dev-dependency (which has a nicer API anyway) instead of adding lazy_static.

I also weakened the guarantee from your test: instead of asserting that the ticker terminates, I think it's strong enough to assert that the ticker is sleeping. At that point, it always only has access to a Weak, so if all strong references to the underlying data have been dropped, the ticker will fail to upgrade and terminate cleanly. I implemented this by changing your AtomicBool to indicate whether the ticker is running, setting to false when the ticker goes to sleep and true when it wakes up again.

This allows us to fix this issue by simply moving the Drop impl from BarState to ProgressBarInner, having it lock the Mutex during drop. This means that:

• If the ticker currently holds the lock, the Drop impl will wait for the ticker to go to sleep.
• If the ticker tries to wake up while the Drop code is running, it will need to wait until Drop drops the lock.
• If the ticker acquires the lock after Drop is run, it will find that state.state.is_finished() and terminate cleanly.
• If the ticker wakes up after all strong references to the ProgressBarInner have dropped, it will terminate cleanly.

I think that covers everything?

[chris-laplante]

Unfortunately this doesn't cover all cases - there is no guarantee that the Drop runs in the context of the main thread (or whatever thread created the ProgressBar) so it is still possible for the program to exit uncleanly (i.e. without the Drop running at all).

Here is a demonstration of the race: https://github.com/chris-laplante/indicatif/tree/cpl/more-barrier-fun

The core issue is the same as before - there is a small window of opportunity where Ticker holds a strong reference to the struct that is supposed to terminate it via Drop. Unless we can guarantee that Ticker never holds the only strong reference, any scheme like this will be vulnerable to a race. I don't think there is any way to structure the code like this, which is why I would suggest the condition variable approach.

[chris-laplante]

I also realize this means my test is not perfect :/.

[djc]

Closing this in favor of #417.