Closed zhangkaizhao closed 1 year ago
Like the documentation says, a ProgressBar
is just a wrapper around an Arc
, so cloning doesn't really create a new ProgressBar
: https://docs.rs/indicatif/latest/indicatif/struct.ProgressBar.html. There might be a way for us the catch this at runtime and prevent the panic, but in general I'd say this isn't a bug in indicatif. Perhaps we could use better documentation here though. @djc what do you think?
I'm a little unclear on why this makes us crash? It seems like it might be reasonable to, on MultiProgress::add()
, do some kind of pointer equality comparison on the Arc<ProgressBar>
and make adding known progress bars a second time a no-op?
In principle I agree that it is the caller's error, but if we can't make it statically impossible (which in this case isn't really possible) it would be nice if at least we make it error in a less painful and/or more obvious way.
The hang occurs because MultiProgress::internalize
takes the MultiState
write lock:
but so does disconnect
:
I can make this a no-op by adding drop(state);
after line 138 in multi.rs. Or, I can make it panic by doing a pointer comparison like @djc suggested.
I can't think of any reason why someone would want to do this purposely, so I'm leaning towards making it panic. Any thoughts?
I mean, if there are no ill effects of doing so than making it a no-op seems better than making it panic. Why crash someone's program if we don't need to?
I mean, if there are no ill effects of doing so than making it a no-op seems better than making it panic. Why crash someone's program if we don't need to?
OK, that's fair. I will make the change and add some documentation for the behavior.
Build pass but dead lock while running. Tested with
indicatif == 0.17.2
.Output while running: