MultiProgress:insert_{after, before}: grab pb index immediately to avoid deadlock with Ticker #424
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before this change it is possible for a combination of Ticker and
MultiProgress to deadlock due a lock inversion. Consider this code:
Inside
Ticker
, a lock onBarState
is acquired and then the state isdrawn to the draw target via
BarState::draw()
.draw
callsself.draw_target.width()
which in the case ofMultiProgress
locksMultiState
.Meanwhile, the call to
insert_after
callsMultiProgress::internalize
which locks the
MultiState
. ThenMultiState::insert
(in the case ofinsert_after
andinsert_before
) locks theBarState
in order toread 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
andinsert_before
to immediatelygrab the index of the progress bar rather than deferring it to be done
when the
MultiState
is already locked. This makes the locking order ofthread 2 the same as that of 1.