Fix relabelling, triple flush and lost text regressions from 4.7.0

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
 ------------------
 

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 <Wolph@Wol.ph> instead.
 
 Please include the progressbar2 version, the configuration that triggers the
-issue and a minimal reproduction. Expect an acknowledgement within a week.
+issue and a minimal reproduction. Expect an acknowledgement within a week.

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.

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()

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']