MultiProgress:insert_{after, before}: grab pb index immediately to av…

…oid deadlock with Ticker (#424)

Before this change it is possible for a combination of Ticker and
MultiProgress to deadlock due a lock inversion. Consider this code:

    let mp = MultiProgress::new();
    let s = mp.add(ProgressBar::new_spinner());
    s.enable_steady_tick(Duration::from_millis(50));

    let p0 = mp.insert_after(&s, ProgressBar::new(10));

Inside `Ticker`, a lock on `BarState` is acquired and then the state is
drawn to the draw target via `BarState::draw()`. `draw` calls
`self.draw_target.width()` which in the case of `MultiProgress` locks
`MultiState`.

Meanwhile, the call to `insert_after` calls `MultiProgress::internalize`
which locks the `MultiState`. Then `MultiState::insert` (in the case of
`insert_after` and `insert_before`) locks the `BarState` in order to
read the index.

So we have thread 1 locking in this order: `BarState`, `MultiState`
and thread 2 locking in this order:        `MultiState`, `BarState`
== boom

The fix is to change `insert_after` and `insert_before` to immediately
grab the index of the progress bar rather than deferring it to be done
when the `MultiState` is already locked. This makes the locking order of
thread 2 the same as that of 1.

src/multi.rs

@@ -94,7 +94,7 @@ impl MultiProgress {
     /// remote draw target that is intercepted by the multi progress
     /// object overriding custom `ProgressDrawTarget` settings.
     pub fn insert_before(&self, before: &ProgressBar, pb: ProgressBar) -> ProgressBar {
-        self.internalize(InsertLocation::Before(before), pb)
+        self.internalize(InsertLocation::Before(before.index().unwrap()), pb)
     }
 
     /// Inserts a progress bar after an existing one.
@@ -103,7 +103,7 @@ impl MultiProgress {
     /// remote draw target that is intercepted by the multi progress
     /// object overriding custom `ProgressDrawTarget` settings.
     pub fn insert_after(&self, after: &ProgressBar, pb: ProgressBar) -> ProgressBar {
-        self.internalize(InsertLocation::After(after), pb)
+        self.internalize(InsertLocation::After(after.index().unwrap()), pb)
     }
 
     /// Removes a progress bar.
@@ -289,13 +289,11 @@ impl MultiState {
                 let pos = self.ordering.len().saturating_sub(pos);
                 self.ordering.insert(pos, idx);
             }
-            InsertLocation::After(after) => {
-                let after_idx = after.index().unwrap();
+            InsertLocation::After(after_idx) => {
                 let pos = self.ordering.iter().position(|i| *i == after_idx).unwrap();
                 self.ordering.insert(pos + 1, idx);
             }
-            InsertLocation::Before(before) => {
-                let before_idx = before.index().unwrap();
+            InsertLocation::Before(before_idx) => {
                 let pos = self.ordering.iter().position(|i| *i == before_idx).unwrap();
                 self.ordering.insert(pos, idx);
             }
@@ -366,12 +364,12 @@ impl Default for MultiProgressAlignment {
     }
 }
 
-enum InsertLocation<'a> {
+enum InsertLocation {
     End,
     Index(usize),
     IndexFromBack(usize),
-    After(&'a ProgressBar),
-    Before(&'a ProgressBar),
+    After(usize),
+    Before(usize),
 }
 
 #[cfg(test)]