From 05b5c13752404bd791e6febb94627d4409a706c0 Mon Sep 17 00:00:00 2001 From: Rick van Hattem Date: Fri, 6 Nov 2026 11:33:14 +0100 Subject: [PATCH 1/2] Add the missing newline at the end of SECURITY.md --- SECURITY.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SECURITY.md b/SECURITY.md index 9fd1e6b..160b015 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -15,4 +15,4 @@ That keeps the report out of the public issue tracker until a fix is available. If you cannot use the form, mail instead. Please include the progressbar2 version, the configuration that triggers the -issue and a minimal reproduction. Expect an acknowledgement within a week. \ No newline at end of file +issue and a minimal reproduction. Expect an acknowledgement within a week. From 9e1fa2ddf178c59dcf0979719096a4de2663e6dc Mon Sep 17 00:00:00 2001 From: Rick van Hattem Date: Fri, 6 Nov 2026 11:33:15 +0100 Subject: [PATCH 2/2] Fix relabelling, triple flush and lost text regressions from 4.7.0 A bar that leaves a MultiBar takes its generated label widgets with it, so adding it again labels it once. The stop_capturing() that brings the count to zero flushes the original stream once, as in 4.6.0. A WrappingIO.flush() replacement that raises no longer costs the captured text. --- CHANGES.rst | 16 +++++++ progressbar/multi.py | 40 ++++++++++++++-- progressbar/utils.py | 67 +++++++++++++++++++------- tests/test_multibar.py | 90 +++++++++++++++++++++++++++++++++++ tests/test_stream_security.py | 75 +++++++++++++++++++++++++++-- 5 files changed, 266 insertions(+), 22 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 403b62e..d93a571 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -2,6 +2,22 @@ Changelog ========= +Unreleased +---------- + +A bar that leaves a ``MultiBar``, through ``pop``, ``del``, ``clear``, +a replacement under the same label or ``remove_finished``, takes its +generated label widgets with it. Adding it again labels it once. 4.7.0 +left the old labels in ``bar.widgets`` and labelled it twice. + +The ``stop_capturing()`` that brings the count to zero flushes the +original stream once, as 4.6.0 did. 4.7.0 flushed it three times. + +A ``WrappingIO.flush()`` replacement that raises no longer costs the +captured text. The text goes back in front of anything written since, +and the next flush delivers it in order. 4.7.0 drained the buffer first +and lost the text. + 4.7.0 - 2026-11-05 ------------------ diff --git a/progressbar/multi.py b/progressbar/multi.py index 4ffd4cb..fc7aa49 100644 --- a/progressbar/multi.py +++ b/progressbar/multi.py @@ -176,6 +176,7 @@ class MultiBar(dict[str, bar.ProgressBar]): _previous_output: list[str] _finished_at: _identity.IdentityMap[bar.ProgressBar, float] _labeled: _identity.IdentitySet[bar.ProgressBar] + _labels: _identity.IdentityMap[bar.ProgressBar, list[str]] _custom_labels: bool _membership_box: utils._SharedLock # pyright: ignore[reportPrivateUsage] _print_lock: threading.RLock @@ -237,6 +238,7 @@ def __init__( self._custom_labels = False self._labeled = _identity.IdentitySet[bar.ProgressBar]() + self._labels = _identity.IdentityMap() self._finished_at = _identity.IdentityMap() self._previous_output = [] self._buffer = io.StringIO() @@ -344,6 +346,21 @@ def _forget_bar(self, progress: bar.ProgressBar | None) -> None: if progress is not None: self._finished_at.pop(progress, None) self._labeled.discard(progress) + self._remove_labels(progress, self._labels.pop(progress, [])) + + @staticmethod + def _remove_labels(progress: bar.ProgressBar, labels: list[str]) -> None: + """Take the generated label widgets out of a bar that leaves. + + The widgets are matched by identity: the bar keeps a label of + its own that happens to render the same text. + """ + widgets: collections.abc.MutableSequence[typing.Any] = progress.widgets + for label in labels: + for index, widget in enumerate(widgets): + if widget is label: + del widgets[index] + break def __delitem__(self, key: str) -> None: """Remove a progressbar and its tracking.""" @@ -424,9 +441,12 @@ def clear(self) -> None: removed = list(super().values()) removed.extend(self._finished_at) removed.extend(self._labeled) + for progress in self._labels: + self._remove_labels(progress, self._labels[progress]) super().clear() self._finished_at.clear() self._labeled.clear() + self._labels.clear() # `removed` keeps the bars alive until this method returns, so # their finalisers run after the membership guard is released. @@ -445,11 +465,25 @@ def _label_bar( if self.prepend_label or self.append_label: self._labeled.add(bar) + labels: list[str] = [] if self.prepend_label: # pragma: no branch - widgets.insert(0, self.label_format.format(label=label)) + prefix: str = self.label_format.format(label=label) + widgets.insert(0, prefix) + labels.append(prefix) if self.append_label: # pragma: no branch - widgets.append(self.label_format.format(label=label)) - return True + suffix: str = self.label_format.format(label=label) + widgets.append(suffix) + labels.append(suffix) + if not labels: + return True + with self._membership_guard(label): + if bar in self._labeled: + self._labels[bar] = labels + return True + # The bar left while its labels were being formatted. Take them + # out again, so a later insertion labels it once. + self._remove_labels(bar, labels) + return not require_member def render(self, flush: bool = True, force: bool = False) -> None: """Redraw every bar, only touching lines that actually changed. diff --git a/progressbar/utils.py b/progressbar/utils.py index 69aaa63..c6e0973 100644 --- a/progressbar/utils.py +++ b/progressbar/utils.py @@ -449,12 +449,33 @@ def _deliver(self, value: str) -> None: if value: # A replacement flush() may log. Call it only during # delivery, outside both the buffer and transition locks. - self.flush() + try: + self.flush() + except BaseException: + # The text was taken already. Put it back in front of + # anything written since, so a flush() that fails once + # (a handler that is not ready yet) costs no output. + self._restore(value) + raise if value and not self.target.closed: self.target.write(value) # when explicitly flushing, always flush the target as well self.flush_target() + def _restore(self, value: str) -> None: + """Put text taken by `_take()` back, ahead of later writes.""" + with ( + self._buffer_box.lock + if self._native_buffer() + else contextlib.nullcontext() + ): + rest: str = self.buffer.getvalue() + self.buffer.seek(0) + self.buffer.truncate(0) + self.buffer.write(value + rest) + if '\n' in value: + self.needs_clear = True + def flush_target(self) -> None: # pragma: no cover """Flush `target` itself, if it's open and flushable.""" if not self.target.closed and getattr(self.target, 'flush', None): @@ -681,6 +702,11 @@ def stop_capturing(self, bar: _ProgressListener | None = None) -> None: """ pending: list[_Pending] = [] failure: BaseException | None = None + # The default hook drains and delivers through `flush()`, as + # 4.6.0 did, so the original stream is flushed once. Only a + # replaced hook, which may not call `super()`, needs this stop + # to deliver its own text before the hook runs. + deliver: bool = not self._default_capture_hook() with self.lock: if bar: # pragma: no branch with self.listeners_lock, contextlib.suppress(KeyError): @@ -690,16 +716,24 @@ def stop_capturing(self, bar: _ProgressListener | None = None) -> None: # the next start_capturing() a no-op self.capturing = max(self.capturing - 1, 0) try: - pending = self._apply_capturing() + if deliver: + pending = self._apply_capturing() + else: + self._propagate_capturing() except BaseException as exception: # noqa: BLE001 failure = exception - if failure is None: + if failure is None and pending: try: self._deliver(pending) except BaseException as exception: # noqa: BLE001 failure = exception self._notify_capturing(failure) + def _default_capture_hook(self) -> bool: + """Return whether `update_capturing` is the one defined here.""" + hook: object = getattr(self.update_capturing, '__func__', None) + return hook is StreamWrapper.update_capturing + def _notify_capturing(self, failure: BaseException | None) -> None: """Notify the public hook without replacing an earlier failure.""" if failure is None: @@ -720,22 +754,28 @@ def update_capturing(self) -> None: # pragma: no cover until something else happens to trigger a flush. Start and stop invoke overrides after the state propagation - attempt. Stop also attempts delivery before invoking the hook, - even if that attempt fails. Calling `super()` reconciles the - current state again and can flush an already empty target. + attempt. A stop with a replaced hook also delivers its own text + before invoking it, even if that delivery fails. Calling + `super()` reconciles the current state again and flushes. Concurrent transitions can change the state before the hook runs, so it can observe a later transition's count and flags. """ with self.lock: - pending: list[_Pending] = self._apply_capturing() - self._deliver(pending) + self._propagate_capturing() if self.capturing <= 0: # 4.6.0 flushed through the public method here, so a # subclass that overrides `flush()` sees the last bar stop - # capturing. The text itself went out above, this drains - # whatever a listener wrote in between. + # capturing, and the original stream is flushed once. self.flush() + def _propagate_capturing(self) -> None: + """Push the capture flag onto the installed wrappers, under `lock`.""" + if isinstance(self.stdout, WrappingIO): + self.stdout.capturing = self.capturing > 0 + + if isinstance(self.stderr, WrappingIO): + self.stderr.capturing = self.capturing > 0 + def _apply_capturing(self) -> list[_Pending]: """Push the capture state onto the wrappers, under `lock`. @@ -748,12 +788,7 @@ def _apply_capturing(self) -> list[_Pending]: same text twice, and an unwrap cannot swap the wrapper out between the type check and the drain. """ - if isinstance(self.stdout, WrappingIO): - self.stdout.capturing = self.capturing > 0 - - if isinstance(self.stderr, WrappingIO): - self.stderr.capturing = self.capturing > 0 - + self._propagate_capturing() if self.capturing > 0: return [] return self._take() diff --git a/tests/test_multibar.py b/tests/test_multibar.py index 20ba30e..7182167 100644 --- a/tests/test_multibar.py +++ b/tests/test_multibar.py @@ -1211,3 +1211,93 @@ def insert() -> None: assert multibar['task'] is progress assert next(iter(multibar)) is label assert observed and all(observed) + + +@pytest.mark.parametrize('removal', ['pop', 'del', 'clear', 'replace']) +@pytest.mark.parametrize( + 'prepend,append', [(True, False), (False, True), (True, True)] +) +def test_reinserted_bar_keeps_one_label_at_each_end( + removal: str, prepend: bool, append: bool +) -> None: + # Regression against 4.7.0: a bar that left the multibar kept its + # generated label widgets, so adding it again labelled it twice. + multibar: progressbar.MultiBar = progressbar.MultiBar( + fd=io.StringIO(), + prepend_label=prepend, + append_label=append, + label_format='{label}:', + ) + bar: progressbar.ProgressBar = progressbar.ProgressBar( + widgets=['body'], max_value=1, fd=io.StringIO() + ) + multibar['task'] = bar + bar.start() + multibar.render() + expected: list[str] = ['body'] + if prepend: + expected.insert(0, 'task:') + if append: + expected.append('task:') + assert bar.widgets == expected + for _ in range(2): + if removal == 'pop': + multibar.pop('task') + elif removal == 'del': + del multibar['task'] + elif removal == 'clear': + multibar.clear() + else: + multibar['task'] = progressbar.ProgressBar(fd=io.StringIO()) + assert bar.widgets == ['body'] + multibar['task'] = bar + multibar.render() + assert bar.widgets == expected + + +def test_bar_removed_while_being_labelled_ends_up_unlabelled() -> None: + # The labels are formatted outside the membership guard, so another + # thread can remove the bar in between. Here the removal happens + # from inside the format call, which pins the same interleaving + # without a tracer (which coverage on 3.10 could not measure). + multibar: progressbar.MultiBar = progressbar.MultiBar( + fd=io.StringIO(), prepend_label=True, label_format='{label}:' + ) + bar: progressbar.ProgressBar = progressbar.ProgressBar( + widgets=['body'], max_value=1, fd=io.StringIO() + ) + + class PoppingFormat(str): + def format(self, *args: object, **kwargs: object) -> str: + multibar.pop('task') + return str.format(self, *args, **kwargs) + + multibar.label_format = PoppingFormat('{label}:') + multibar['task'] = bar + assert multibar._label_bar(bar, require_member=True) is False + assert 'task' not in multibar + assert bar.widgets == ['body'] + assert bar not in multibar._labeled + assert bar not in multibar._labels + multibar.label_format = '{label}:' + multibar['task'] = bar + assert multibar._label_bar(bar, require_member=True) is True + assert bar.widgets == ['task:', 'body'] + + +def test_label_already_removed_by_the_caller_costs_no_other_widget() -> None: + multibar: progressbar.MultiBar = progressbar.MultiBar( + fd=io.StringIO(), prepend_label=True, label_format='{label}:' + ) + bar: progressbar.ProgressBar = progressbar.ProgressBar( + widgets=['task:', 'body'], max_value=1, fd=io.StringIO() + ) + multibar['task'] = bar + bar.start() + multibar.render() + assert bar.widgets == ['task:', 'task:', 'body'] + del bar.widgets[0] + multibar.pop('task') + # Only the generated label object is removed, matched by identity, + # so the bar's own equal-looking label stays. + assert bar.widgets == ['task:', 'body'] diff --git a/tests/test_stream_security.py b/tests/test_stream_security.py index 4c752c4..5645718 100644 --- a/tests/test_stream_security.py +++ b/tests/test_stream_security.py @@ -1117,9 +1117,10 @@ def check_locks() -> None: assert streams.capturing == 0 assert stdout.capturing is False and stderr.capturing is False assert err.getvalue() == 'eggs' - assert out.getvalue() == ( - 'spam' if failure_at in ('target_flush', 'drain') else '' - ) + # Only a failed target write costs the text, so a retry cannot + # write it twice. A failed buffer flush keeps it for the hook's + # own flush, as in 4.6.0. + assert out.getvalue() == ('' if failure_at == 'write' else 'spam') streams.unwrap_stdout() streams.unwrap_stderr() @@ -1466,3 +1467,71 @@ def update_capturing(self) -> None: # the stop that brought the count to zero. assert calls == ['update_capturing', 'update_capturing', 'flush'] assert target.getvalue() == 'spam' + + +def test_default_capture_stop_flushes_the_target_once() -> None: + # 4.6.0 flushed the original stream once when the last bar stopped + # capturing. 4.7.0 flushed it three times: from stop_capturing() + # and twice more from the default update_capturing() it then ran. + class Target(io.StringIO): + flushes: int = 0 + + def flush(self) -> None: + self.flushes += 1 + super().flush() + + target: Target = Target() + streams: utils.StreamWrapper = utils.StreamWrapper() + streams.stdout = utils.WrappingIO(target) + streams.wrapped_stdout = 1 + streams.start_capturing() + streams.stdout.write('spam') + assert target.flushes == 0 + streams.stop_capturing() + assert target.getvalue() == 'spam' + assert target.flushes == 1 + + +@pytest.mark.parametrize('shadow', [False, True]) +def test_failing_flush_hook_keeps_the_captured_text(shadow: bool) -> None: + # A flush() replacement that raises once must not cost the text + # already taken from the buffer. 4.6.0 called flush() before + # draining, 4.7.0 drained first and lost the text. + failure: OSError = OSError('temporary flush failure') + attempts: int = 0 + + def flush() -> None: + nonlocal attempts + attempts += 1 + if attempts == 1: + raise failure + + class RetryFlush(utils.WrappingIO): + def flush(self) -> None: + flush() + + target: io.StringIO = io.StringIO() + healthy: io.StringIO = io.StringIO() + wrapper: utils.WrappingIO = ( + utils.WrappingIO(target, capturing=True) + if shadow + else RetryFlush(target, capturing=True) + ) + if shadow: + wrapper.flush = flush # type: ignore[method-assign] + streams: utils.StreamWrapper = utils.StreamWrapper() + streams.stdout = wrapper + streams.stderr = utils.WrappingIO(healthy, capturing=True) + streams.wrapped_stdout = streams.wrapped_stderr = 1 + wrapper.write('first|\n') + streams.stderr.write('healthy|') + with pytest.raises(OSError) as caught: + streams.flush() + assert caught.value is failure + assert healthy.getvalue() == 'healthy|' + assert wrapper.buffer.getvalue() == 'first|\n' + assert wrapper.needs_clear is True + wrapper.write('second|') + streams.flush() + assert target.getvalue() == 'first|\nsecond|' + assert wrapper.buffer.getvalue() == ''