diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 934fe608..83b75401 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -10,6 +10,9 @@ on: required: true type: string +permissions: + contents: read + jobs: build: name: Build distributions @@ -18,15 +21,71 @@ jobs: - uses: actions/checkout@v6 with: ref: ${{ inputs.ref || github.ref }} + - name: Verify the ref is a release tag matching __version__ + env: + EVENT_NAME: ${{ github.event_name }} + EVENT_REF: ${{ github.ref }} + INPUT_REF: ${{ inputs.ref }} + run: | + set -euo pipefail + if [ "$EVENT_NAME" = "workflow_dispatch" ]; then + ref="$INPUT_REF" + else + ref="$EVENT_REF" + fi + case "$ref" in + refs/tags/v*) tag="${ref#refs/tags/}" ;; + v*) + if [ "$EVENT_NAME" != "workflow_dispatch" ]; then + echo "::error::ref '$ref' is not a tag ref" + exit 1 + fi + tag="$ref" + ;; + *) + echo "::error::ref '$ref' is not a v* release tag" + exit 1 + ;; + esac + if ! [[ "$tag" =~ ^v[0-9]+\.[0-9]+\.[0-9]+[0-9A-Za-z.+-]*$ ]]; then + echo "::error::tag '$tag' does not look like vX.Y.Z" + exit 1 + fi + version="$(sed -n -E "s/^__version__(: *str)? = ['\"]([^'\"]*)['\"].*/\2/p" progressbar/__about__.py)" + if [ -z "$version" ]; then + echo "::error::could not read __version__ from progressbar/__about__.py" + exit 1 + fi + if [ "$tag" != "v$version" ]; then + echo "::error::tag '$tag' does not match __version__ '$version'" + exit 1 + fi + tag_commit="$(git rev-parse --verify --quiet "refs/tags/$tag^{commit}")" || { + echo "::error::tag '$tag' is not present in the checkout" + exit 1 + } + if [ "$tag_commit" != "$(git rev-parse HEAD)" ]; then + echo "::error::checked-out commit is not the commit of tag '$tag'" + exit 1 + fi + echo "Releasing $tag ($tag_commit)" - name: Install uv uses: astral-sh/setup-uv@v7 - name: Build sdist and wheel run: uv build --no-sources - - name: Verify wheel contains only the progressbar package + - name: Verify distribution contents run: | + set -euo pipefail unpacked="$(unzip -Z1 dist/*.whl | cut -d/ -f1 | sort -u)" echo "$unpacked" test "$(echo "$unpacked" | grep -cvE '^progressbar$|^progressbar2-.*\.dist-info$')" -eq 0 + sdist="$(tar tzf dist/*.tar.gz)" + echo "$sdist" + stray_pattern='\.sync-conflict-|\.(orig|rej|bak)$' + if { unzip -Z1 dist/*.whl; echo "$sdist"; } | grep -E "$stray_pattern"; then + echo "::error::distribution contains stray conflict or backup files" + exit 1 + fi - uses: actions/upload-artifact@v6 with: name: dist diff --git a/.gitignore b/.gitignore index ccdf6182..1e2abf8a 100644 --- a/.gitignore +++ b/.gitignore @@ -48,3 +48,11 @@ docs/_build/ # Built at docs build time for the in-browser console docs/_static/wheels/ + +# Local secrets and environments +.env +.env.* +.venv/ +*.pem +*.key +.pypirc diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 1d38c796..b92dc8aa 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -5,6 +5,8 @@ repos: rev: v6.0.0 hooks: - id: end-of-file-fixer + # Preserve the upstream bytes copied by the vendor update command. + exclude: '^docs/_static/vendor/xterm\.js$' - id: trailing-whitespace - id: check-yaml - id: check-added-large-files diff --git a/CHANGES.rst b/CHANGES.rst index 4e5c6c0c..bf81de12 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -2,6 +2,39 @@ Changelog ========= +Unreleased +---------- + +The ``progressbar`` and ``bar`` commands, and ``python -m progressbar``, +now exit with status 130 after Ctrl-C interrupts a transfer. Previously +they exited with status 0. Scripts that check the exit status can now +distinguish an interrupted transfer from a completed copy. + +Captured output now survives a drain failure on the other stream, and +unwrapping restores the original stream even when its capture buffer is +closed. Built-in ``StringIO`` capture writes and drains remain atomic. +Caller-replaced buffers keep their method overrides, which run without +stream locks. As in 4.6.0, callers synchronise concurrent writes and drains +on those custom buffers. A capture-hook failure now retains the other +stream's exception in the primary failure's context chain. + +Specialised format templates retain their escaping and result types. +Their field paths, including nested format specifications, are checked +before calling the application's formatter. + +MultiBar expiry keeps a replacement registered under the same exact +``str`` label. Assignment, update, in-place union and ordinary removal +methods share its membership guard. Lifecycle tracking uses bar identity +without calling application hash or equality methods, and the guard resets +after fork. +Custom label objects retain their hash and equality hooks and their +identity as dictionary keys. Using one for mutation or indexing switches +that MultiBar instance permanently to the 4.6.0 concurrency contract: +callers synchronise concurrent mutation and rendering, including expiry. +These hooks run without the membership guard, even for later exact +``str`` lookups. Clearing the instance does not reset this mode. Direct +calls to ``dict`` base-class mutation methods bypass this protocol. + For the most recent changes to the Python Progressbar please look at the Git releases or the commit log: diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 5477d533..a8ba6047 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -118,14 +118,20 @@ Ready to contribute? Here's how to set up `python-progressbar` for local develop Now you can make your changes locally. -5. When you're done making changes, check that your changes pass the - linter and the tests, including other Python versions with tox:: +5. Before committing, format the changed Python files, then check the + whole tree and run the tests, including other Python versions with tox:: - $ uv run ruff check progressbar tests + $ uvx ruff@latest format path/to/changed.py + $ uvx ruff@latest check . + $ uvx ruff@latest format --check . $ uv run pytest $ uv run tox - The ``uv sync`` from step 3 already installed all of these. + Stage the formatted changes before committing. A checkout-local + pre-commit gate, when installed, checks the staged snapshot with current + Ruff and the configured pre-commit hooks, including the CI-pinned Ruff + version. It leaves partially staged files untouched. Configured fixes + run only in the disposable snapshot and cause the commit to be rejected. 6. Commit your changes and push your branch to GitHub with `git-flow-avh`_:: diff --git a/benchmarks/results.json b/benchmarks/results.json index 62515853..83c64411 100644 --- a/benchmarks/results.json +++ b/benchmarks/results.json @@ -114,4 +114,4 @@ } } } -} \ No newline at end of file +} diff --git a/progressbar/__main__.py b/progressbar/__main__.py index f271b08f..bc37530a 100644 --- a/progressbar/__main__.py +++ b/progressbar/__main__.py @@ -9,12 +9,13 @@ import argparse import contextlib +import os import pathlib import sys import time import typing from pathlib import Path -from typing import IO, BinaryIO, TextIO +from typing import IO import progressbar @@ -386,118 +387,318 @@ def _sleep_for_rate_limit( time.sleep(delay) -def main(argv: list[str] | None = None) -> None: # noqa: C901 +#: Exit status after an interrupted transfer (128 + SIGINT, as shells do). +EXIT_INTERRUPTED: int = 130 + + +def _parse_size_option( + parser: argparse.ArgumentParser, + option: str, + value: str, + minimum: int, +) -> int: + """Parse a size option, exiting through `parser.error` when invalid. + + Args: + parser: The parser used to report the error (exit status 2). + option: The flag name, used in the error message. + value: The raw option value. + minimum: The smallest accepted value in bytes. + + Returns: + The size in bytes. + """ + try: + size: int = size_to_bytes(value) + except (ValueError, IndexError, OSError) as exception: + parser.error(f'invalid {option} value {value!r}: {exception}') + if size < minimum: + parser.error(f'{option} must be at least {minimum}, got {value!r}') + if size > sys.maxsize: + parser.error(f'{option} is too large: {value!r}') + return size + + +def _collect_inputs( + parser: argparse.ArgumentParser, + args: argparse.Namespace, +) -> tuple[list[IO[typing.Any] | Path], int, bool]: + """Validate the input arguments without opening any file. + + Returns: + The inputs (stdin streams or paths), the summed file size and + whether a total size is known. + """ + inputs: list[IO[typing.Any] | Path] = [] + total_size: int = 0 + filesize_available: bool = True + for filename in args.input: + if filename == '-': + if sys.stdin is None or sys.stdin.closed: + parser.error('stdin is closed, cannot read from -') + stdin: IO[typing.Any] = ( + sys.stdin if args.line_mode else sys.stdin.buffer + ) + if stdin.closed: + parser.error('stdin is closed, cannot read from -') + inputs.append(stdin) + filesize_available = False + continue + + input_path: Path = pathlib.Path(filename) + if not input_path.exists(): + parser.error(f'File not found: {filename}') + if input_path.is_dir(): + parser.error(f'Is a directory: {filename}') + if not os.access(input_path, os.R_OK): + parser.error(f'Permission denied: {filename}') + total_size += input_path.stat().st_size + inputs.append(input_path) + return inputs, total_size, filesize_available + + +def _check_output_path( + parser: argparse.ArgumentParser, + output: str | None, + inputs: typing.Sequence[IO[typing.Any] | Path], +) -> None: + """Reject an output path that is a directory or one of the inputs. + + Runs before the output is opened, because opening it truncates it. + """ + if not output or output == '-': + return + output_path: Path = pathlib.Path(output) + if not output_path.exists(): + return + if output_path.is_dir(): + parser.error(f'Output is a directory: {output}') + if not output_path.is_file(): + # Devices (two names of one tty, /dev/null) share an inode without + # sharing data, so only a regular file can be truncated away + return + output_stat: os.stat_result = output_path.stat() + for input_item in inputs: + same: bool = ( + output_path.samefile(input_item) + if isinstance(input_item, Path) + else _stream_is_file(input_item, output_stat) + ) + if same: + parser.error(f'Output file is also an input: {output}') + + +def _stream_is_file(stream: IO[typing.Any], target: os.stat_result) -> bool: + """Return whether `stream` (stdin, say) is open on the file `target`. + + A stream without a descriptor (`io.BytesIO`, a test double) is not + an alias of anything. + """ + try: + stream_stat: os.stat_result = os.fstat(stream.fileno()) + except (AttributeError, OSError, ValueError): + return False + return (stream_stat.st_dev, stream_stat.st_ino) == ( + target.st_dev, + target.st_ino, + ) + + +def _open_input( + input_path: IO[typing.Any] | Path, + line_mode: bool, +) -> typing.ContextManager[IO[typing.Any]]: + """Open a path input, or wrap a stdin stream without closing it.""" + if isinstance(input_path, Path): + if line_mode: + # newline='' disables universal-newline translation so the + # byte count matches the file size for CRLF files as well + return input_path.open('r', newline='') + return input_path.open('rb') + return contextlib.nullcontext(input_path) + + +def _copy_stream( + input_stream: IO[typing.Any], + output_stream: IO[typing.Any], + bar: progressbar.ProgressBar, + line_mode: bool, + buffer_size: int, + rate_limit: int | None, + started_at: float, + transferred: int, +) -> int: + """Copy one input to the output, updating `bar`. + + Returns: + The running total of bytes transferred. + """ + while True: + data: str | bytes + if line_mode: + data = input_stream.readline(buffer_size) + else: + data = input_stream.read(buffer_size) + + if not data: + return transferred + + output_stream.write(data) + if isinstance(data, str): + # The total size is measured in bytes, so progress must be + # tracked in bytes as well + encoding: str = getattr(input_stream, 'encoding', None) or 'utf-8' + transferred += len(data.encode(encoding, errors='replace')) + else: + transferred += len(data) + + bar.update(transferred) + _sleep_for_rate_limit(rate_limit, transferred, started_at) + + +def _output_is_stdout(output_stream: typing.IO[typing.Any]) -> bool: + """Return whether `output_stream` writes to the same place as stdout. + + `sys.stdout` and its binary buffer count, and so does a file opened + by `-o` on a path that aliases it (`/dev/stdout`, `/dev/fd/1`): its + descriptor differs, but `fstat` reports the same device and inode. + """ + if output_stream is sys.stdout or output_stream is getattr( + sys.stdout, 'buffer', None + ): + return True + try: + stdout_stat: os.stat_result = os.fstat(sys.stdout.fileno()) + except (AttributeError, OSError, ValueError): + return False + return _stream_is_file(output_stream, stdout_stat) + + +def _fileno(stream: typing.IO[typing.Any]) -> int | None: + """Return the descriptor behind `stream`, or None when it has none. + + An `io.StringIO` under a test runner has no descriptor, and a closed + file raises `ValueError` for one. + """ + try: + return stream.fileno() + except (OSError, ValueError, AttributeError): + return None + + +def _discard_broken_stdout(output_stream: typing.IO[typing.Any]) -> None: + """Point fd 1 (and an `-o` alias of it) at `os.devnull` after EPIPE. + + The copy loop has already stopped, but the interpreter flushes + `sys.stdout` once more at exit, and that flush would raise the same + `BrokenPipeError` again (status 120, "Exception ignored on flushing + sys.stdout"). Redirecting the descriptor makes that last flush + succeed, which is what the Python docs recommend for a pipeline + whose reader went away. A file opened on `/dev/stdout` is closed by + the exit stack with the same failing flush, so its descriptor is + redirected as well. + """ + descriptors: set[int] = { + fd + for fd in (_fileno(sys.stdout), _fileno(output_stream)) + if fd is not None + } + # A replacement stdout that owns another descriptor is left alone + if 1 not in descriptors: + return + try: + devnull: int = os.open(os.devnull, os.O_WRONLY) + try: + for fd in descriptors: + os.dup2(devnull, fd) + finally: + os.close(devnull) + except OSError: # pragma: no cover + pass + + +def main(argv: list[str] | None = None) -> None: """Run the `progressbar` command. + All arguments are validated before the output file is opened, so an + invalid invocation exits with status 2 and leaves `-o` untouched. An + interrupted transfer (Ctrl-C) exits with status 130 after the bar + is finished. A downstream pipe that closes early ends the copy + quietly with status 0, so `... | head` stays clean under pipefail. + Args: argv: Command-line arguments. Defaults to `sys.argv[1:]`. """ parser: argparse.ArgumentParser = create_argument_parser() args: argparse.Namespace = parser.parse_args(argv) + inputs, total_size, filesize_available = _collect_inputs(parser, args) + if args.size: + total_size = _parse_size_option(parser, '--size', args.size, 0) + filesize_available = True + buffer_size: int = ( + _parse_size_option(parser, '--buffer-size', args.buffer_size, 1) + if args.buffer_size + else 1024 + ) + rate_limit: int | None = ( + _parse_size_option(parser, '--rate-limit', args.rate_limit, 0) + if args.rate_limit + else None + ) + _check_output_path(parser, args.output, inputs) + + widgets = _build_widgets(args, filesize_available) + progress_bar_class: type[progressbar.ProgressBar] = ( + progressbar.NullBar if args.quiet else progressbar.ProgressBar + ) + exit_status: int = 0 + with contextlib.ExitStack() as stack: output_stream: typing.IO[typing.Any] = _get_output_stream( args.output, args.line_mode, stack ) - - input_paths: list[BinaryIO | TextIO | Path | IO[typing.Any]] = [] - total_size: int = 0 - filesize_available: bool = True - for filename in args.input: - input_path: typing.IO[typing.Any] | pathlib.Path - if filename == '-': - if args.line_mode: - input_path = sys.stdin - else: - input_path = sys.stdin.buffer - - filesize_available = False - else: - input_path = pathlib.Path(filename) - if not input_path.exists(): - parser.error(f'File not found: {filename}') - - if not args.size: - total_size += input_path.stat().st_size - - input_paths.append(input_path) - - # Determine the size for the progress bar (if provided) - if args.size: - total_size = size_to_bytes(args.size) - filesize_available = True - - widgets = _build_widgets(args, filesize_available) - progress_bar_class: type[progressbar.ProgressBar] = ( - progressbar.NullBar if args.quiet else progressbar.ProgressBar - ) - bar = progress_bar_class( widgets=widgets, max_value=total_size if filesize_available else None, max_error=False, line_breaks=True if args.numeric else None, ) - - buffer_size = ( - size_to_bytes(args.buffer_size) if args.buffer_size else 1024 - ) - rate_limit = ( - size_to_bytes(args.rate_limit) if args.rate_limit else None - ) - started_at = time.monotonic() - total_transferred = 0 + started_at: float = time.monotonic() + transferred: int = 0 bar.start() - with contextlib.suppress(KeyboardInterrupt, BrokenPipeError): - for input_path in input_paths: - if isinstance(input_path, pathlib.Path): - if args.line_mode: - # newline='' disables universal-newline - # translation so the byte count matches the file - # size for CRLF files as well - input_stream = stack.enter_context( - input_path.open('r', newline=''), - ) - else: - input_stream = stack.enter_context( - input_path.open('rb'), - ) - else: - input_stream = input_path - - while True: - data: str | bytes - if args.line_mode: - data = input_stream.readline(buffer_size) - else: - data = input_stream.read(buffer_size) - - if not data: - break - - output_stream.write(data) - if isinstance(data, str): - # The total size is measured in bytes, so progress - # must be tracked in bytes as well - encoding = ( - getattr(input_stream, 'encoding', None) or 'utf-8' - ) - total_transferred += len( - data.encode(encoding, errors='replace'), - ) - else: - total_transferred += len(data) - - bar.update(total_transferred) - _sleep_for_rate_limit( + try: + for input_path in inputs: + with _open_input(input_path, args.line_mode) as input_stream: + transferred = _copy_stream( + input_stream, + output_stream, + bar, + args.line_mode, + buffer_size, rate_limit, - total_transferred, started_at, + transferred, ) + # A reader that went away is only noticed when the buffer + # reaches the pipe, which for a small copy is this flush, so + # it must happen inside the guarded block + output_stream.flush() + except KeyboardInterrupt: + exit_status = EXIT_INTERRUPTED + except BrokenPipeError: + # The reader went away (`... | head`): finish quietly with + # status 0, as 4.6.0 did and as a pipeline under pipefail + # expects + if _output_is_stdout(output_stream): + _discard_broken_stdout(output_stream) bar.finish(dirty=True) + if exit_status: + raise SystemExit(exit_status) + def _get_output_stream( output: str | None, diff --git a/progressbar/_identity.py b/progressbar/_identity.py new file mode 100644 index 00000000..b7245c94 --- /dev/null +++ b/progressbar/_identity.py @@ -0,0 +1,79 @@ +"""Private identity-based tracking that never calls application hash hooks.""" + +from __future__ import annotations + +import collections.abc +import typing + +_Key = typing.TypeVar('_Key') +_Value = typing.TypeVar('_Value') + + +class IdentityMap(collections.abc.MutableMapping[_Key, _Value]): + """Track bars by identity without calling their hash or equality hooks.""" + + def __init__(self) -> None: + self._items: dict[int, tuple[_Key, _Value]] = {} + + def __getstate__(self) -> list[tuple[_Key, _Value]]: + """Serialise retained objects without their process-local ids.""" + return list(self._items.values()) + + def __setstate__(self, state: list[tuple[_Key, _Value]]) -> None: + """Rebuild ids after pickle or deepcopy restores the objects.""" + self._items = {id(key): (key, value) for key, value in state} + + def __getitem__(self, key: _Key) -> _Value: + """Return the value belonging to this exact bar.""" + return self._items[id(key)][1] + + def __setitem__(self, key: _Key, value: _Value) -> None: + """Retain the bar alongside its value to prevent id reuse.""" + self._items[id(key)] = (key, value) + + def __delitem__(self, key: _Key) -> None: + """Remove this exact bar's entry.""" + del self._items[id(key)] + + def __iter__(self) -> collections.abc.Iterator[_Key]: + """Iterate the retained bars.""" + return (item[0] for item in self._items.values()) + + def __len__(self) -> int: + """Return the number of tracked bars.""" + return len(self._items) + + +class IdentitySet(collections.abc.MutableSet[_Key]): + """Remember labelled bars without calling application hash hooks.""" + + def __init__(self) -> None: + self._items: dict[int, _Key] = {} + + def __getstate__(self) -> list[_Key]: + """Serialise retained objects without their process-local ids.""" + return list(self._items.values()) + + def __setstate__(self, state: list[_Key]) -> None: + """Rebuild ids after pickle or deepcopy restores the objects.""" + self._items = {id(value): value for value in state} + + def __contains__(self, value: object) -> bool: + """Check whether this exact object is remembered.""" + return id(value) in self._items + + def __iter__(self) -> collections.abc.Iterator[_Key]: + """Iterate remembered bars.""" + return iter(self._items.values()) + + def __len__(self) -> int: + """Return the number of remembered bars.""" + return len(self._items) + + def add(self, value: _Key) -> None: + """Retain a labelled bar by identity.""" + self._items[id(value)] = value + + def discard(self, value: _Key) -> None: + """Forget a labelled bar if it was remembered.""" + self._items.pop(id(value), None) diff --git a/progressbar/_parallel/_display.py b/progressbar/_parallel/_display.py index d481a31a..20e9ba6f 100644 --- a/progressbar/_parallel/_display.py +++ b/progressbar/_parallel/_display.py @@ -15,6 +15,7 @@ from __future__ import annotations import os +import re import sys import typing @@ -28,6 +29,22 @@ if typing.TYPE_CHECKING: from .. import multi as multi_module +# Batch items are data, not caller-authored markup. Remove every +# ESC-introduced sequence and every C0/C1 control (CR, LF and TAB +# included) so the label is inert text on exactly one terminal row. +_LABEL_CONTROL_RE: re.Pattern[str] = re.compile( + r'\x1b\[[0-?]*[ -/]*[@-~]' # CSI + r'|\x1b\][^\x07\x1b\x9c]*(?:\x07|\x1b\\|\x9c)?' # OSC + r'|\x1b[PX^_][^\x1b\x9c]*(?:\x1b\\|\x9c)?' # DCS, SOS, PM, APC + r'|\x1b[ -/]*[0-~]?' # other ESC sequences + r'|[\x00-\x1f\x7f-\x9f]' # C0, DEL, C1 +) + + +def _display_label(label: str) -> str: + """Return a batch-item label as inert, single-row display text.""" + return _LABEL_CONTROL_RE.sub('', label) + @typing.runtime_checkable class Display(typing.Protocol): @@ -251,7 +268,7 @@ def task_started( self, seq: int, label: str ) -> bar_module.ProgressBar | None: """Add a per-task bar and hand it out for `current_task_bar`.""" - key: str = f'{seq}: {label}' + key: str = f'{seq}: {_display_label(str(label))}' self._keys[seq] = key task_bar: bar_module.ProgressBar = bar_module.ProgressBar( max_value=base.UnknownLength, @@ -261,10 +278,18 @@ def task_started( return task_bar def task_finished(self, seq: int, ok: bool) -> None: - """Drop the task's bar -- finished bars must not pile up.""" + """Drop and finish the task's bar. + + Finished bars must not pile up in the MultiBar, and each bar + must be finished so its stream listeners, capture refcounts and + resize-handler registration return to their baseline. + """ key: str | None = self._keys.pop(seq, None) - if key is not None and key in self.multibar: - del self.multibar[key] + if key is None or key not in self.multibar: + return + task_bar: bar_module.ProgressBar = self.multibar[key] + del self.multibar[key] + task_bar.finish(end='', dirty=not ok) def advance(self, n: int = 1) -> None: """Count completions on the overall bar. diff --git a/progressbar/_parallel/_shell.py b/progressbar/_parallel/_shell.py index bc747788..04ce6ba8 100644 --- a/progressbar/_parallel/_shell.py +++ b/progressbar/_parallel/_shell.py @@ -10,6 +10,7 @@ import functools import os +import re import shlex import subprocess import typing @@ -19,6 +20,10 @@ #: The two placeholder spellings recognized in command templates. _PLACEHOLDERS: tuple[str, str] = ('{}', '{item}') +#: Matches either placeholder spelling, so a template is substituted in +#: one pass and placeholder text inside an item is never re-expanded. +_PLACEHOLDER_RE: re.Pattern[str] = re.compile(r'\{(?:item)?\}') + #: A command template: a string, an argv list, or a callable that #: builds the argv for one item. CommandT = ( @@ -29,10 +34,21 @@ def _substitute(token: str, item_text: str) -> str: - """Replace the placeholder spellings inside one token.""" - for placeholder in _PLACEHOLDERS: - token = token.replace(placeholder, item_text) - return token + """Replace the placeholder spellings inside one token in one pass.""" + return _PLACEHOLDER_RE.sub(lambda _match: item_text, token) + + +def _check_shell(command: CommandT, *, shell: bool) -> None: + """Reject ``shell=True`` with a list or callable template. + + `subprocess.run` with a sequence and ``shell=True`` runs only the + first element through the shell and drops the rest, item included. + """ + if shell and not isinstance(command, str): + raise TypeError( + 'shell=True requires a str command template, got ' + f'{type(command).__name__}' + ) def _has_placeholder(tokens: typing.Iterable[str]) -> bool: @@ -66,7 +82,11 @@ def build_argv( Returns: The argv list, or the command string when ``shell=True``. + + Raises: + TypeError: ``shell=True`` with a list or callable template. """ + _check_shell(command, shell=shell) item_text: str = str(item) if callable(command): return [str(part) for part in command(item)] @@ -120,6 +140,7 @@ def make_runner( env: typing.Any = None, ) -> typing.Callable[[typing.Any], subprocess.CompletedProcess[typing.Any]]: """Bind a command template into a per-item callable for `map`.""" + _check_shell(command, shell=shell) return functools.partial( _run_one, command, diff --git a/progressbar/bar.py b/progressbar/bar.py index 3cb8d963..5e70a7a5 100644 --- a/progressbar/bar.py +++ b/progressbar/bar.py @@ -41,6 +41,28 @@ ) from .terminal import os_specific +_PACKAGE_DIR: str = os.path.dirname(os.path.abspath(__file__)) + + +def _caller_stacklevel() -> int: + """Return the `stacklevel` that points a warning at the first frame + outside progressbar. + + `warnings.warn()` counts level 1 as the function that calls it, so the + walk starts at this helper's caller and climbs until the code is not + ours. A fixed level would break as soon as a subclass adds an + `__init__` between the user and `ProgressBar.__init__`. + """ + frame: FrameType | None = sys._getframe(1) + level: int = 1 + while frame is not None and frame.f_code.co_filename.startswith( + _PACKAGE_DIR + ): + frame = frame.f_back + level += 1 + return level + + try: # Optional native accelerator, shipped as the ``progressbar2[fast]`` extra # (the separate ``speedups`` package). When importable, the iterator path @@ -54,6 +76,56 @@ _FastBarIterator = None +_bad_interval_seen: set[str] = set() +#: A shared lock box, so a forked child gets an unlocked lock +_bad_interval_lock: utils._SharedLock = utils._SharedLock() + + +def _reset_bad_interval_warnings() -> None: + """Forget the bad values warned about so far (for tests).""" + with _bad_interval_lock.lock: + _bad_interval_seen.clear() + + +def _warn_bad_minimum_update_interval(raw: str) -> None: + """Warn once per distinct bad value, not once per bar. + + The value is recorded under a lock before the warning goes out, so + two bars constructed at the same moment on different threads + produce one record, and the warning itself is logged outside the + lock. Every distinct bad value seen in the process is remembered. + """ + with _bad_interval_lock.lock: + if raw in _bad_interval_seen: + return + _bad_interval_seen.add(raw) + logger.warning( + 'Ignoring PROGRESSBAR_MINIMUM_UPDATE_INTERVAL=%r: ' + 'not a non-negative finite number', + raw, + ) + + +def _env_minimum_update_interval() -> float: + """Read `PROGRESSBAR_MINIMUM_UPDATE_INTERVAL`, ignoring bad values. + + A non-numeric, negative or non-finite value is logged as a warning + and falls back to 0, so a stray environment variable cannot stop a + bar from being built. An unset or empty variable is 0 silently. + """ + raw: str = os.environ.get('PROGRESSBAR_MINIMUM_UPDATE_INTERVAL', '') + if not raw.strip(): + return 0.0 + try: + value: float = float(raw) + except ValueError: + value = -1.0 + if not math.isfinite(value) or value < 0: + _warn_bad_minimum_update_interval(raw) + return 0.0 + return value + + @functools.cache def _load_widgets() -> typing.Any: """Import the widgets module lazily (and once). @@ -518,18 +590,38 @@ class _ResizeRegistry: A single signal handler dispatches to every live bar. The original handler is saved when the first bar registers and restored when the last one unregisters, so overlapping bars can finish in any order - without leaving a dangling handler installed. + without leaving a dangling handler installed. A bar that is garbage + collected without `finish()` counts as unregistered too. + + `signal.signal` only works on the main thread, so a last bar + finished elsewhere leaves the shared handler in place with nobody + to dispatch to. The saved handler is kept for that case, and the + restore is retried at the next registration change or the next + resize signal (which does run on the main thread). Whether the + shared handler is installed is read from `signal.getsignal` rather + than remembered, so a handler that other code set over it is never + clobbered by a restore. """ bars: typing.ClassVar[weakref.WeakSet[ResizableMixin]] = weakref.WeakSet() previous_handler: typing.ClassVar[typing.Any] = None + @classmethod + def installed(cls) -> bool: + """Return whether `handle_resize` is the handler set on SIGWINCH.""" + import signal + + return ( + signal.getsignal(signal.SIGWINCH) # type: ignore[attr-defined] + == cls.handle_resize + ) + @classmethod def install(cls, bar: ResizableMixin) -> None: """Register `bar` for resize dispatch, installing the handler once. The process-global `SIGWINCH` handler is only installed (saving - whatever handler was previously set) on the first registration. A + whatever handler was previously set) when it is not ours yet. A second, third, etc. overlapping bar just joins the `WeakSet`. """ import signal @@ -538,7 +630,7 @@ def install(cls, bar: ResizableMixin) -> None: # Not available on Windows return - if not cls.bars: + if not cls.installed(): cls.previous_handler = signal.getsignal( signal.SIGWINCH # type: ignore[attr-defined] ) @@ -548,6 +640,7 @@ def install(cls, bar: ResizableMixin) -> None: ) cls.bars.add(bar) + weakref.finalize(bar, cls._restore_if_idle) @classmethod def uninstall(cls, bar: ResizableMixin) -> None: @@ -558,27 +651,65 @@ def uninstall(cls, bar: ResizableMixin) -> None: keep the shared handler installed regardless of registration order. """ + cls.bars.discard(bar) + cls._restore_if_idle() + + @classmethod + def _restore_if_idle(cls) -> None: + """Put the saved handler back when no live bar is registered. + + Iterating the `WeakSet` skips entries whose bar is already dead + but not yet removed, which is the state inside a finalizer. + """ import signal if not hasattr(signal, 'SIGWINCH'): # pragma: no cover # Not available on Windows return - - cls.bars.discard(bar) - if not cls.bars: + if not cls.installed() or any(True for _ in cls.bars): + return + handler = cls.previous_handler + if handler is None: + # `getsignal` returns None for a handler installed from C, + # which cannot be reinstalled from Python. SIGWINCH is + # ignored by default, so the default is the closest fit. + handler = signal.SIG_DFL + try: signal.signal( signal.SIGWINCH, # type: ignore[attr-defined] - cls.previous_handler, + handler, ) - cls.previous_handler = None + except ValueError: + # Not the main thread: keep `previous_handler` and retry at + # the next registration change or resize signal + return + cls.previous_handler = None @classmethod def handle_resize( cls, signum: int | None = None, frame: FrameType | None = None ) -> None: - """Dispatch a `SIGWINCH` to every currently-registered bar.""" + """Dispatch a `SIGWINCH` to every currently-registered bar. + + A bar whose `_handle_resize` raises is logged and skipped, so + the other bars still get the resize and nothing propagates out + of the signal handler into unrelated main-thread code. + """ for bar in list(cls.bars): + cls._dispatch(bar, signum, frame) + cls._restore_if_idle() + + @staticmethod + def _dispatch( + bar: ResizableMixin, signum: int | None, frame: FrameType | None + ) -> None: + """Resize one bar, logging instead of raising when it fails.""" + try: bar._handle_resize(signum, frame) + except Exception: + logger.warning( + 'Ignoring a failed resize of %r', bar, exc_info=True + ) class ResizableMixin(ProgressBarMixinBase): @@ -764,7 +895,9 @@ class ProgressBar( line_offset: The number of lines to offset the progressbar from your current line. This is useful if you have other output or multiple progressbars. - desc: tqdm-style alias for `prefix` (rendered as `f'{desc}: '`). + desc: tqdm-style description, rendered literally as + `f'{desc}: '` in front of the bar. Unlike `prefix` it is + never a format template: braces in a file name stay braces. Ignored if `prefix` is also given. total: tqdm-style alias for `max_value`. Ignored if `max_value` is also given. @@ -849,9 +982,14 @@ def __init__( if max_value is None and total is not None: # tqdm-style alias for `max_value` max_value = total + self._literal_prefix: str | None = None if prefix is None and desc is not None: - # tqdm-style alias for `prefix` + # tqdm-style alias for `prefix`: `desc` is literal text, as in + # tqdm and as the fast path renders it. Remember the exact + # string, so `_init_prefix` escapes its braces and a prefix + # assigned later is a template again. prefix = f'{desc}: ' + self._literal_prefix = prefix if max_value and min_value > typing.cast(NumberT, max_value): raise ValueError( @@ -904,7 +1042,7 @@ def _apply_deprecated_aliases( 'The usage of `maxval` is deprecated, please use ' '`max_value` instead', DeprecationWarning, - stacklevel=1, + stacklevel=_caller_stacklevel(), ) max_value = kwargs.get('maxval') @@ -913,7 +1051,7 @@ def _apply_deprecated_aliases( 'The usage of `poll` is deprecated, please use ' '`poll_interval` instead', DeprecationWarning, - stacklevel=1, + stacklevel=_caller_stacklevel(), ) poll_interval = kwargs.get('poll') @@ -964,7 +1102,7 @@ def _setup_poll_intervals( self.min_poll_interval = max( min_poll_interval or self._MINIMUM_UPDATE_INTERVAL, self._MINIMUM_UPDATE_INTERVAL, - float(os.environ.get('PROGRESSBAR_MINIMUM_UPDATE_INTERVAL', 0)), + _env_minimum_update_interval(), ) # type: ignore def _seed_variables(self, variables: dict[str, typing.Any] | None) -> None: @@ -1665,9 +1803,12 @@ def _init_prefix(self) -> None: if self.prefix: widgets = _load_widgets() + template: str = self.prefix + if template is getattr(self, '_literal_prefix', None): + # A desc is literal text: escape its braces for the label + template = template.replace('{', '{{').replace('}', '}}') self.widgets.insert( - 0, - widgets.FormatLabel(self.prefix, new_style=True), + 0, widgets.FormatLabel(template, new_style=True) ) self.prefix = None @@ -1738,7 +1879,7 @@ def currval(self) -> NumberT: warnings.warn( 'The usage of `currval` is deprecated, please use `value` instead', DeprecationWarning, - stacklevel=1, + stacklevel=_caller_stacklevel(), ) return self.value diff --git a/progressbar/env.py b/progressbar/env.py index 7c81c8d1..2b7c7dcb 100644 --- a/progressbar/env.py +++ b/progressbar/env.py @@ -162,10 +162,11 @@ def is_ansi_terminal( PyCharm terminal (not under pytest) short-circuits straight to `True`, since both render ANSI without being a tty. Otherwise, `fd.isatty()` plus a `TERM` match against `ANSI_TERM_RE`, or - `ANSICON` being set, or (on Windows) the console-mode probe (see - `os_specific.windows.get_console_mode` for what that probe actually - tests, which is not what its flag name suggests). Only the errors a - stream can legitimately raise while being probed are swallowed: + `ANSICON` being set, or (on Windows, for a tty only) the + console-mode probe (see `os_specific.windows.get_console_mode` for + what that probe actually tests, which is not what its flag name + suggests). Only the errors a stream can legitimately raise while + being probed are swallowed: `OSError` (real I/O), `ValueError` (closed/detached file) and `AttributeError` (no `isatty` at all). Anything else is a bug and propagates. @@ -199,7 +200,7 @@ def is_ansi_terminal( # ANSICON is a Windows ANSI compatible console elif 'ANSICON' in os.environ: is_terminal = True - elif os.name == 'nt': + elif os.name == 'nt' and is_tty: from .terminal.os_specific import windows return bool( diff --git a/progressbar/multi.py b/progressbar/multi.py index d3a4b6af..8bdd53fd 100644 --- a/progressbar/multi.py +++ b/progressbar/multi.py @@ -10,6 +10,7 @@ from __future__ import annotations import collections.abc +import contextlib import enum import importlib import io @@ -25,7 +26,7 @@ import python_utils -from . import bar, terminal +from . import _identity, bar, terminal, utils from .terminal import stream # MultiBar renders full (widget) progress bars from background threads. Warm @@ -37,6 +38,7 @@ importlib.import_module('progressbar.widgets') SortKeyFunc = collections.abc.Callable[[bar.ProgressBar], typing.Any] +_NO_LABEL: object = object() class _Update(typing.Protocol): @@ -81,6 +83,15 @@ class MultiBar(dict[str, bar.ProgressBar]): program can exit. The default preserves the historical wait-forever behavior. + Note: + Exact ``str`` labels retain atomic membership changes and expiry. + Using a custom label for mutation or indexing permanently switches + this instance to the 4.6.0 concurrency contract. Its hash and + equality hooks run without the membership guard, and the caller + synchronises concurrent mutation and rendering. Original key + objects and dictionary equality semantics are retained. Direct + ``dict`` base-method mutation bypasses this protocol. + Note: `fd` is resolved once, from the parameter default, at the time this module is first imported. Unlike a plain `ProgressBar`, @@ -163,8 +174,10 @@ class MultiBar(dict[str, bar.ProgressBar]): sort_keyfunc: SortKeyFunc _previous_output: list[str] - _finished_at: dict[bar.ProgressBar, float] - _labeled: set[bar.ProgressBar] + _finished_at: _identity.IdentityMap[bar.ProgressBar, float] + _labeled: _identity.IdentitySet[bar.ProgressBar] + _custom_labels: bool + _membership_box: utils._SharedLock # pyright: ignore[reportPrivateUsage] _print_lock: threading.RLock _thread: threading.Thread | None _thread_finished: threading.Event @@ -222,10 +235,14 @@ def __init__( self.sort_keyfunc = sort_keyfunc self.sort_reverse = sort_reverse - self._labeled = set() - self._finished_at = {} + self._custom_labels = False + self._labeled = _identity.IdentitySet[bar.ProgressBar]() + self._finished_at = _identity.IdentityMap() self._previous_output = [] self._buffer = io.StringIO() + self._membership_box = utils._SharedLock( # pyright: ignore[reportPrivateUsage] + threading.RLock + ) self._print_lock = threading.RLock() self._thread = None self._thread_finished = threading.Event() @@ -247,6 +264,29 @@ def __init__( for key, progress in bar_items: self[key] = progress + @property + def _membership_lock(self) -> threading.RLock: + """Return the membership guard, reset automatically after fork.""" + return self._membership_box.lock + + @contextlib.contextmanager + def _membership_guard( + self, key: object = _NO_LABEL + ) -> collections.abc.Generator[None, None, None]: + """Guard native labels and hand custom labels back to their caller. + + The switch waits for previous native operations to finish. Every + waiting operation checks the mode after acquisition. Custom mode + stays active after clear because earlier callbacks may still run. + """ + with self._membership_lock: + if key is not _NO_LABEL and type(key) is not str: + self._custom_labels = True + if not self._custom_labels: + yield + return + yield + def __setitem__(self, key: str, bar: bar.ProgressBar) -> None: """Add `bar` to the multibar, rebinding it to draw through us. @@ -276,57 +316,138 @@ def __setitem__(self, key: str, bar: bar.ProgressBar) -> None: key: The label and dict key to add/update `bar` under. bar: The progressbar to add, mutated as described above. """ - if bar.label != key or not key: # pragma: no branch - bar.label = key - + self._prepare_bar(key, bar) + with self._membership_guard(key): + previous: bar.ProgressBar | None = super().get(key) + super().__setitem__(key, bar) + if previous is not bar: + self._forget_bar(previous) + + def _prepare_bar(self, key: str, progress: bar.ProgressBar) -> None: + """Configure a bar outside the membership guard.""" + if progress.label != key or not key: # pragma: no branch + progress.label = key if not ( - isinstance(bar.fd, stream.LastLineStream) - and bar.fd.stream is self.fd + isinstance(progress.fd, stream.LastLineStream) + and progress.fd.stream is self.fd ): - bar.fd = stream.LastLineStream(self.fd) - - bar.paused = True - # `mypy` rejects assigning to a method, hence the ignore. - bar.print = self.print # type: ignore - - # Just in case someone is using a progressbar with a custom - # constructor and forgot to call the super constructor - if bar.index == -1: - bar.index = next( - bar._index_counter # pyright: ignore[reportPrivateUsage] + progress.fd = stream.LastLineStream(self.fd) + progress.paused = True + progress.print = self.print # type: ignore + if progress.index == -1: + progress.index = next( + progress._index_counter # pyright: ignore[reportPrivateUsage] ) - super().__setitem__(key, bar) + def _forget_bar(self, progress: bar.ProgressBar | None) -> None: + """Discard tracking within the current membership contract.""" + if progress is not None: + self._finished_at.pop(progress, None) + self._labeled.discard(progress) def __delitem__(self, key: str) -> None: - """Remove a progressbar from the multibar.""" - bar_: bar.ProgressBar = self.pop(key) - self._finished_at.pop(bar_, None) - self._labeled.discard(bar_) + """Remove a progressbar and its tracking.""" + self.pop(key) def __getitem__(self, key: str) -> bar.ProgressBar: """Get (and create if needed) a progressbar from the multibar.""" - try: - return super().__getitem__(key) - except KeyError: - progress = bar.ProgressBar(**self.progressbar_kwargs) + with self._membership_guard(key): + try: + return super().__getitem__(key) + except KeyError: + pass + progress: bar.ProgressBar = bar.ProgressBar(**self.progressbar_kwargs) + return self.setdefault(key, progress) + + def update(self, *args: typing.Any, **kwargs: bar.ProgressBar) -> None: + """Add mapping or iterable entries through normal bar setup.""" + entries: dict[str, bar.ProgressBar] = dict(*args, **kwargs) + for key, progress in entries.items(): self[key] = progress - return progress - def _label_bar(self, bar: bar.ProgressBar) -> None: - """Insert the label widget(s) into `bar.widgets`, once per bar.""" - if bar in self._labeled: # pragma: no branch - return + def __ior__(self, other: typing.Any) -> MultiBar: + """Update in place using the same membership guard as assignment.""" + self.update(other) + return self + + @typing.overload + def setdefault( + self, key: str, default: bar.ProgressBar + ) -> bar.ProgressBar: ... + + @typing.overload + def setdefault( + self, key: str, default: None = None + ) -> bar.ProgressBar | None: ... + + def setdefault( + self, key: str, default: bar.ProgressBar | None = None + ) -> bar.ProgressBar | None: + """Insert a missing member without replacing a concurrent one.""" + with self._membership_guard(key): + if super().__contains__(key): + return super().__getitem__(key) + if default is not None: + self._prepare_bar(key, default) + with self._membership_guard(key): + members: dict[str, bar.ProgressBar | None] = typing.cast( + dict[str, bar.ProgressBar | None], self + ) + return dict[str, bar.ProgressBar | None].setdefault( + members, key, default + ) - assert bar.widgets, 'Cannot prepend label to empty progressbar' + def pop(self, key: str, *args: typing.Any) -> typing.Any: + """Remove a member and discard its lifecycle tracking.""" + with self._membership_guard(key): + present: bool = super().__contains__(key) + progress: typing.Any = super().pop(key, *args) + if present: + self._forget_bar(progress) + return progress - if self.prepend_label: # pragma: no branch - self._labeled.add(bar) - bar.widgets.insert(0, self.label_format.format(label=bar.label)) + def popitem(self) -> tuple[str, bar.ProgressBar]: + """Remove the newest member and its tracking.""" + with self._membership_guard(): + key: str + progress: bar.ProgressBar + key, progress = super().popitem() + self._forget_bar(progress) + return key, progress + + def clear(self) -> None: + """Remove all members and their tracking.""" + removed: list[bar.ProgressBar] + with self._membership_guard(): + removed = list(super().values()) + removed.extend(self._finished_at) + removed.extend(self._labeled) + super().clear() + self._finished_at.clear() + self._labeled.clear() + # Release the last references after leaving the membership guard. + del removed + + def _label_bar( + self, bar: bar.ProgressBar, *, require_member: bool = False + ) -> bool: + """Reserve a bar's label under the guard, then format it unlocked.""" + label: str = bar.label + widgets: collections.abc.MutableSequence[typing.Any] = bar.widgets + assert widgets, 'Cannot prepend label to empty progressbar' + with self._membership_guard(label): + if require_member and super().get(label) is not bar: + return False + if bar in self._labeled: # pragma: no branch + return True + if self.prepend_label or self.append_label: + self._labeled.add(bar) + if self.prepend_label: # pragma: no branch + widgets.insert(0, self.label_format.format(label=label)) if self.append_label: # pragma: no branch - self._labeled.add(bar) - bar.widgets.append(self.label_format.format(label=bar.label)) + widgets.append(self.label_format.format(label=label)) + return True def render(self, flush: bool = True, force: bool = False) -> None: """Redraw every bar, only touching lines that actually changed. @@ -418,7 +539,8 @@ def _render_bar( def update( force: bool = True, write: bool = True ) -> str: # pragma: no cover - self._label_bar(bar_) + if not self._label_bar(bar_, require_member=True): + return '' bar_.update(force=force) if write: return typing.cast(stream.LastLineStream, bar_.fd).line @@ -456,17 +578,27 @@ def _render_finished_bar( Returns: Zero lines (dropped or hidden) or one rendered line. """ - if bar_ not in self._finished_at: - self._finished_at[bar_] = now - # Force update to get the finished format + label: str = bar_.label + with self._membership_guard(label): + if super().get(label) is not bar_: + return + finished_at: float | None = self._finished_at.get(bar_) + first_finish: bool = finished_at is None + if finished_at is None: + finished_at = self._finished_at.setdefault(bar_, now) + if first_finish: + # Widgets and targets may call back into membership methods. update(write=False) if ( self.remove_finished and expired is not None - and expired >= self._finished_at[bar_] + and expired >= finished_at ): - del self[bar_.label] + with self._membership_guard(label): + if super().get(label) is bar_: + super().__delitem__(label) + self._forget_bar(bar_) return if not self.show_finished: @@ -662,10 +794,11 @@ def get_sorted_bars(self) -> list[bar.ProgressBar]: The bars sorted by `sort_keyfunc`, reversed if `sort_reverse`. The values are copied into a list first so a concurrent `__setitem__`/`__delitem__` from another - thread (the multibar is a plain `dict`, not a thread-safe - one) can't mutate it out from under the sort. + thread cannot mutate it during the snapshot. Sorting runs + after releasing the membership guard. """ - bars = list(self.values()) + with self._membership_guard(): + bars: list[bar.ProgressBar] = list(super().values()) return sorted(bars, key=self.sort_keyfunc, reverse=self.sort_reverse) def __enter__(self) -> MultiBar: diff --git a/progressbar/utils.py b/progressbar/utils.py index af9c6aec..18695fca 100644 --- a/progressbar/utils.py +++ b/progressbar/utils.py @@ -14,11 +14,14 @@ import contextlib import datetime import io +import itertools import logging import os import re import sys +import threading import typing +import weakref from collections.abc import Callable, Iterable, Iterator, Mapping from types import TracebackType @@ -170,6 +173,69 @@ def len_color(value: types.StringTypes) -> int: return len(no_color(value)) +#: One installed wrapper's drained text, waiting to be written to the +#: original stream once the transition lock is released. +_Drain = str | BaseException | None +_Pending = tuple[str, 'WrappingIO', _Drain] + +if typing.TYPE_CHECKING: + _Lock = threading.Lock +else: + #: `threading.Lock` is a factory function before Python 3.13, so an + #: annotation that names it does not evaluate there + #: (`typing.get_type_hints` raises). The instance type does. + _Lock = type(threading.Lock()) + + +class _SharedLock: + """A lock that several objects share, and that survives pickle and fork. + + `StreamWrapper` and every `WrappingIO` it installs hold the same + box, so they agree on one lock however the graph is pickled or + deep-copied (the box is one object, restored once, whichever object + is the root). The lock itself is never pickled: a restored box gets + a fresh one. Every live box is registered so a forked child can + replace the lock it inherited, which another parent thread may + have been holding at fork time and would otherwise stay locked + forever in the child. + """ + + __slots__ = ('__weakref__', '_factory', 'lock') + + def __init__( + self, + factory: Callable[[], typing.Any] = threading.Lock, + lock: typing.Any = None, + ) -> None: + self._factory: Callable[[], typing.Any] = factory + self.lock: typing.Any = lock if lock is not None else factory() + _lock_boxes.add(self) + + def reset(self) -> None: + """Replace the lock with a new, unlocked one (after a fork).""" + self.lock = self._factory() + + def __getstate__(self) -> Callable[[], typing.Any]: + return self._factory + + def __setstate__(self, factory: Callable[[], typing.Any]) -> None: + self._factory = factory + self.lock = factory() + _lock_boxes.add(self) + + +_lock_boxes: weakref.WeakSet[_SharedLock] = weakref.WeakSet() + + +def _reset_locks_after_fork() -> None: # pragma: no cover + for box in list(_lock_boxes): + box.reset() + + +if hasattr(os, 'register_at_fork'): # pragma: no branch + os.register_at_fork(after_in_child=_reset_locks_after_fork) + + class WrappingIO: """`sys.stdout`/`sys.stderr` replacement installed while capturing. @@ -183,13 +249,33 @@ class WrappingIO: target: base.IO capturing: bool listeners: set[_ProgressListener] + #: Holds the lock that guards `listeners` only. The class-level + #: default keeps a subclass that skips `super().__init__()` working, + #: as for `StreamWrapper`. + _lock_box: _SharedLock = _SharedLock() + #: Holds the lock that keeps `write()` and `_take()` from + #: interleaving on `buffer`: a write between the read and the + #: truncate of a drain was lost. Held around the `io.StringIO` + #: write, getvalue, seek and truncate calls only, never while + #: calling a listener, `target` or a custom buffer method. + _buffer_box: _SharedLock = _SharedLock(threading.RLock) needs_clear: bool = False + @property + def lock(self) -> _Lock: + """The lock guarding `listeners`, shared with the installer.""" + return self._lock_box.lock + + @lock.setter + def lock(self, lock: _Lock) -> None: + self._lock_box = _SharedLock(threading.Lock, lock) + def __init__( self, target: base.IO, capturing: bool = False, listeners: set[_ProgressListener] | None = None, + lock: _Lock | _SharedLock | None = None, ) -> None: """Wrap `target` so writes can be buffered while `capturing`. @@ -202,11 +288,24 @@ def __init__( write completes a line. Typically shared with `StreamWrapper.listeners` by the caller, so every wrapped stream notifies the same bars. + lock: The lock that guards the `listeners` set and nothing + else. `StreamWrapper` passes its `listeners_lock`, so + the snapshot taken in `write()` cannot race with + `start_capturing()` and `stop_capturing()` on another + thread (a real race on free-threaded builds). It is + never held while calling out, so it cannot take part + in a lock-order cycle with a logging handler or a + locking target stream. Defaults to a private lock. """ self.buffer = io.StringIO() self.target = target self.capturing = capturing - self.listeners = listeners or set() + self.listeners = listeners if listeners is not None else set() + if isinstance(lock, _SharedLock): + self._lock_box = lock + else: + self._lock_box = _SharedLock(threading.Lock, lock) + self._buffer_box = _SharedLock(threading.RLock) self.needs_clear = False def write(self, value: str) -> int: @@ -227,10 +326,20 @@ def write(self, value: str) -> int: """ ret = 0 if self.capturing: - ret += self.buffer.write(value) + with ( + self._buffer_box.lock + if self._native_buffer() + else contextlib.nullcontext() + ): + ret += self.buffer.write(value) if '\n' in value: # pragma: no branch self.needs_clear = True - for listener in self.listeners: # pragma: no branch + # Iterate a snapshot taken under the lock: a listener's + # update() may finish its bar, which removes it from this + # shared set, and another thread may be mutating it + with self.lock: + listeners = tuple(self.listeners) + for listener in listeners: # pragma: no branch listener.update() else: ret += self.target.write(value) @@ -258,16 +367,68 @@ def _flush(self) -> None: end, even when the buffer was empty, since this runs on every bar redraw, not just when there's something to flush. """ - if value := self.buffer.getvalue(): - self.flush() - # Clear the buffer before writing so a failed write cannot - # cause the same data to be written again by the next flush - self.buffer.seek(0) - self.buffer.truncate(0) - self.needs_clear = False - if not self.target.closed: - self.target.write(value) + self._deliver(self._take()) + + def _take(self) -> str: + """Drain and return the buffered text without touching `target`. + Native StringIO drains hold the buffer lock. `StreamWrapper` + also captures them under its transition lock. Custom buffer + methods run unlocked and retain caller-owned synchronisation. + """ + with ( + self._buffer_box.lock + if self._native_buffer() + else contextlib.nullcontext() + ): + value: str = self.buffer.getvalue() + if value: + # Clear the buffer before writing so a failed write + # cannot cause the same data to be written again by the + # next flush + self.buffer.seek(0) + self.buffer.truncate(0) + self.needs_clear = False + return value + + def _native_buffer(self) -> bool: + """Recognise built-in operations, including instance shadowing. + + Custom buffers keep their original method dispatch. Their callers + own synchronisation of concurrent writes and drains, as in 4.6.0. + """ + return type(self.buffer) is io.StringIO and not any( + name in vars(self.buffer) + for name in ('write', 'getvalue', 'seek', 'truncate', 'flush') + ) + + def _capture(self) -> _Drain: + """Take native text or its failure now, defer custom code.""" + if ( + not self._native_buffer() + or type(self)._take is not WrappingIO._take + or '_take' in vars(self) + ): + return None + try: + return self._take() + except BaseException as exception: # noqa: BLE001 + return exception + + def _deliver_capture(self, captured: _Drain) -> None: + """Drain deferred custom text and deliver outside stream locks.""" + if isinstance(captured, BaseException): + raise captured + self._deliver(self._take() if captured is None else captured) + + def _deliver(self, value: str) -> None: + """Write text taken by `_take()` to `target`, then flush it.""" + if value: + # A replacement flush() may log. Call it only during + # delivery, outside both the buffer and transition locks. + self.flush() + if value and not self.target.closed: + self.target.write(value) # when explicitly flushing, always flush the target as well self.flush_target() @@ -351,6 +512,31 @@ def __exit__( self.close() +def _link_exception_context( + primary: BaseException, + secondary: BaseException, + *, + prepend: bool = False, +) -> None: + """Retain distinct context objects once, preserving hook precedence.""" + heads: tuple[BaseException | None, BaseException | None] = ( + (secondary, primary.__context__) + if prepend + else (primary.__context__, secondary) + ) + chain: list[BaseException] = [primary] + seen: set[int] = {id(primary)} + for head in heads: + current: BaseException | None = head + while current is not None and id(current) not in seen: + chain.append(current) + seen.add(id(current)) + current = current.__context__ + for previous, following in itertools.pairwise(chain): + previous.__context__ = following + chain[-1].__context__ = None + + class StreamWrapper: """Wrap `sys.stdout`/`sys.stderr` for output, logging, and a bar to share. @@ -374,10 +560,32 @@ class StreamWrapper: wrapped_stdout: int = 0 wrapped_stderr: int = 0 wrapped_logging: int = 0 - wrapped_excepthook: int = 0 + wrapped_excepthook: bool = False logging_handlers: list[tuple[logging.StreamHandler[base.IO], base.IO]] capturing: int = 0 listeners: set[_ProgressListener] + #: Holds the lock guarding every wrap/unwrap/capture transition. + #: Reentrant, because wrap_stdout() calls wrap_excepthook() while + #: holding it. Nothing is logged and no original stream is written + #: while it is held, so it cannot take part in a lock-order cycle + #: with a logging handler or a locking target stream. The + #: class-level default keeps a subclass that skips + #: `super().__init__()` working, as every counter above. + _lock_box: _SharedLock = _SharedLock(threading.RLock) + #: Holds the lock guarding `listeners` only. Shared with every + #: `WrappingIO` this wrapper installs, and never held while calling + #: out. + _listeners_box: _SharedLock = _SharedLock() + + @property + def lock(self) -> threading.RLock: + """The reentrant lock guarding wrap/unwrap/capture transitions.""" + return self._lock_box.lock + + @property + def listeners_lock(self) -> _Lock: + """The lock guarding `listeners`, shared with the wrappers.""" + return self._listeners_box.lock def __init__(self) -> None: """Capture the *current* `sys.stdout`/`sys.stderr` as "real". @@ -397,10 +605,12 @@ def __init__(self) -> None: self.wrapped_stdout = 0 self.wrapped_stderr = 0 self.wrapped_logging = 0 - self.wrapped_excepthook = 0 + self.wrapped_excepthook = False self.logging_handlers = [] self.capturing = 0 self.listeners = set() + self._lock_box = _SharedLock(threading.RLock) + self._listeners_box = _SharedLock() if env.env_flag('WRAP_STDOUT', default=False): # pragma: no cover self.wrap_stdout() @@ -411,30 +621,72 @@ def __init__(self) -> None: def start_capturing(self, bar: _ProgressListener | None = None) -> None: """Turn capturing on for `bar` and bump the shared refcount. + Notify `update_capturing()` outside the locks after propagating + the new state. If propagation fails after the count changes, + still notify the hook and preserve the propagation failure. + Args: bar: Registered as a listener so it's notified (`update()`) when captured output completes a line. Omit to just bump the refcount without listening. """ - if bar: # pragma: no branch - self.listeners.add(bar) - - self.capturing += 1 - self.update_capturing() + failure: BaseException | None = None + with self.lock: + if bar: # pragma: no branch + with self.listeners_lock: + self.listeners.add(bar) + + self.capturing += 1 + # Never flushes here: the count just went above zero + try: + self._apply_capturing() + except BaseException as exception: # noqa: BLE001 + failure = exception + self._notify_capturing(failure) def stop_capturing(self, bar: _ProgressListener | None = None) -> None: """Unregister `bar` and drop the shared capturing refcount. + Notify `update_capturing()` outside the locks after propagating + state and attempting delivery of this stop's owned text. The + hook still runs if propagation or delivery fails. The original + failure takes precedence over an additional hook failure. + Args: bar: The listener to remove, if it was registered via `start_capturing`. """ - if bar: # pragma: no branch - with contextlib.suppress(KeyError): - self.listeners.remove(bar) - - self.capturing -= 1 - self.update_capturing() + pending: list[_Pending] = [] + failure: BaseException | None = None + with self.lock: + if bar: # pragma: no branch + with self.listeners_lock, contextlib.suppress(KeyError): + self.listeners.remove(bar) + + # Never go negative: an unmatched stop would otherwise make + # the next start_capturing() a no-op + self.capturing = max(self.capturing - 1, 0) + try: + pending = self._apply_capturing() + except BaseException as exception: # noqa: BLE001 + failure = exception + if failure is None: + try: + self._deliver(pending) + except BaseException as exception: # noqa: BLE001 + failure = exception + self._notify_capturing(failure) + + def _notify_capturing(self, failure: BaseException | None) -> None: + """Notify the public hook without replacing an earlier failure.""" + if failure is None: + self.update_capturing() + else: + try: + self.update_capturing() + except BaseException as exception: # noqa: BLE001 + _link_exception_context(failure, exception, prepend=True) + raise failure def update_capturing(self) -> None: # pragma: no cover """Propagate the capturing refcount to the wrapped streams. @@ -443,6 +695,29 @@ def update_capturing(self) -> None: # pragma: no cover so whatever's left in the buffer reaches the terminal as soon as the last bar stops capturing, rather than sitting there 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. + 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) + + def _apply_capturing(self) -> list[_Pending]: + """Push the capture state onto the wrappers, under `lock`. + + Returns the buffered text taken from each installed wrapper + when the refcount reached zero, for the caller to deliver after + releasing `lock`: a delivery writes into the original streams, + and one of those may be a logging handler or a widget that + takes a lock of its own on another thread. Taking the text + under the lock means two overlapping flushes never deliver the + 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 @@ -450,8 +725,70 @@ def update_capturing(self) -> None: # pragma: no cover if isinstance(self.stderr, WrappingIO): self.stderr.capturing = self.capturing > 0 - if self.capturing <= 0: - self.flush() + if self.capturing > 0: + return [] + return self._take() + + def _take(self) -> list[_Pending]: + """Capture native text or failures and defer custom drains.""" + pending: list[_Pending] = [] + if self.wrapped_stdout and isinstance(self.stdout, WrappingIO): + pending.append(('stdout', self.stdout, self.stdout._capture())) + if self.wrapped_stderr and isinstance(self.stderr, WrappingIO): + pending.append(('stderr', self.stderr, self.stderr._capture())) + return pending + + def _deliver(self, pending: list[_Pending]) -> None: + """Write taken text to the original streams, outside `lock`. + + If writing to a stream's target raises `io.UnsupportedOperation` + (as happens for some non-seekable streams), that stream's + redirection disables itself: `wrapped_stdout`/`wrapped_stderr` + is reset to 0 so no further flush attempts are made, and a + warning is logged. `sys.stdout`/`sys.stderr` are left installed + as-is: only further flush attempts are skipped, not the wrapping + itself. + + Any other exception from a stream propagates to the caller, but + only after the other stream's text has been delivered too: the + text was already taken from the buffers, so a stdout that fails + must not cost the stderr lines of the same round. + """ + failure: BaseException | None = None + for name, wrapper, text in pending: + exception: BaseException | None = self._deliver_guarded( + name, wrapper, text + ) + if failure is None: + failure = exception + elif exception is not None and exception is not failure: + _link_exception_context(failure, exception) + if failure is not None: + raise failure + + def _deliver_guarded( + self, name: str, wrapper: WrappingIO, text: _Drain + ) -> BaseException | None: + """Deliver one wrapper's text, returning what it raised.""" + try: + self._deliver_one(name, wrapper, text) + except BaseException as exception: # noqa: BLE001 + return exception + return None + + def _deliver_one( + self, name: str, wrapper: WrappingIO, text: _Drain + ) -> None: + """Deliver one wrapper's text, disabling its redirection on failure.""" + try: + wrapper._deliver_capture(text) + except io.UnsupportedOperation: + setattr(self, f'wrapped_{name}', 0) + logger.warning( + 'Disabling %s redirection, %r is not seekable', + name, + getattr(sys, name), + ) def wrap(self, stdout: bool = False, stderr: bool = False) -> None: """Wrap `stdout` and/or `stderr`, per the given flags.""" @@ -475,16 +812,20 @@ def wrap_stdout(self) -> WrappingIO: The installed `WrappingIO`, the same instance on every call until it's fully unwrapped. """ - self.wrap_excepthook() - - if not self.wrapped_stdout: - self.stdout = sys.stdout = WrappingIO( # type: ignore - self.original_stdout, - listeners=self.listeners, - ) - self.wrapped_stdout += 1 + with self.lock: + hooked: bool = self._wrap_excepthook_locked() - return sys.stdout # type: ignore + if not self.wrapped_stdout: + self.stdout = sys.stdout = WrappingIO( # type: ignore + self.original_stdout, + listeners=self.listeners, + lock=self._listeners_box, + ) + self.wrapped_stdout += 1 + wrapper: WrappingIO = sys.stdout # type: ignore + if hooked: + self._log_excepthook_wrapped() + return wrapper def wrap_stderr(self) -> WrappingIO: """Install a `WrappingIO` over `sys.stderr`, or share it. @@ -496,16 +837,20 @@ def wrap_stderr(self) -> WrappingIO: The installed `WrappingIO`, the same instance on every call until it's fully unwrapped. """ - self.wrap_excepthook() + with self.lock: + hooked: bool = self._wrap_excepthook_locked() - if not self.wrapped_stderr: - self.stderr = sys.stderr = WrappingIO( # type: ignore - self.original_stderr, - listeners=self.listeners, - ) - self.wrapped_stderr += 1 - - return sys.stderr # type: ignore + if not self.wrapped_stderr: + self.stderr = sys.stderr = WrappingIO( # type: ignore + self.original_stderr, + listeners=self.listeners, + lock=self._listeners_box, + ) + self.wrapped_stderr += 1 + wrapper: WrappingIO = sys.stderr # type: ignore + if hooked: + self._log_excepthook_wrapped() + return wrapper def wrap_logging(self) -> None: """Retarget every `StreamHandler` in the logger tree to the wrapper. @@ -634,9 +979,10 @@ def _iter_loggers(self) -> types.Iterator[logging.Logger]: def unwrap_excepthook(self) -> None: """Restore the original `sys.excepthook`, if currently wrapped.""" - if self.wrapped_excepthook: - self.wrapped_excepthook -= 1 - sys.excepthook = self.original_excepthook + with self.lock: + if self.wrapped_excepthook: + self.wrapped_excepthook = False + sys.excepthook = self.original_excepthook def wrap_excepthook(self) -> None: """Install the shared excepthook that flushes buffered output. @@ -645,10 +991,28 @@ def wrap_excepthook(self) -> None: both call this unconditionally, and either one may already have wrapped it. """ - if not self.wrapped_excepthook: - logger.debug('wrapping excepthook') - self.wrapped_excepthook += 1 - sys.excepthook = self.excepthook + with self.lock: + hooked: bool = self._wrap_excepthook_locked() + if hooked: + self._log_excepthook_wrapped() + + def _wrap_excepthook_locked(self) -> bool: + """Install the excepthook under `lock`; True when newly installed.""" + if self.wrapped_excepthook: + return False + self.wrapped_excepthook = True + sys.excepthook = self.excepthook + return True + + @staticmethod + def _log_excepthook_wrapped() -> None: + """Log the hook installation, after `lock` has been released. + + Logging while holding `lock` could wait for a handler lock held + by a thread that is writing into a wrapper and waiting for + `lock` in turn. + """ + logger.debug('wrapping excepthook') def unwrap(self, stdout: bool = False, stderr: bool = False) -> None: """Unwrap `stdout` and/or `stderr`, per the given flags.""" @@ -658,6 +1022,39 @@ def unwrap(self, stdout: bool = False, stderr: bool = False) -> None: if stderr: self.unwrap_stderr() + @staticmethod + def _drop(dropped: object) -> tuple[WrappingIO, _Drain] | None: + """Stop buffering in a dropped wrapper and take what it holds. + + Runs under `lock`; the text goes to `_deliver_dropped` after + the lock is released. Returns None for a plain stream. + """ + if not isinstance(dropped, WrappingIO): + return None + dropped.capturing = False + return dropped, dropped._capture() + + @staticmethod + def _deliver_dropped(taken: tuple[WrappingIO, _Drain] | None) -> None: + """Deliver the text `_drop` took, outside `lock`. + + A stale reference to the wrapper must not swallow output. The + delivery touches the original stream, so a stream that cannot + take it (no `flush()`, closed, a broken pipe, a GUI widget that + is gone) is logged and otherwise ignored: unwrapping never + raises for it, as it did not before the flush was added. + """ + if taken is None: + return + dropped, text = taken + try: + dropped._deliver_capture(text) + except Exception: + logger.debug( + 'Could not deliver buffered output on unwrap', + exc_info=True, + ) + def unwrap_stdout(self) -> None: """Undo one `wrap_stdout()` call, restoring at refcount zero. @@ -668,15 +1065,23 @@ def unwrap_stdout(self) -> None: excepthook once `stderr` is back to its original too, since it's shared between the two. """ - if self.wrapped_stdout > 1: - self.wrapped_stdout -= 1 - else: - # Also reset our own reference so needs_clear() and - # update_capturing() don't act on a stale wrapper - self.stdout = sys.stdout = self.original_stdout - self.wrapped_stdout = 0 - if not self.wrapped_stderr: - self.unwrap_excepthook() + with self.lock: + if self.wrapped_stdout > 1: + self.wrapped_stdout -= 1 + return + + try: + taken = self._drop(self.stdout) + finally: + # Also reset our own reference so needs_clear() and + # update_capturing() don't act on a stale wrapper + self.stdout = sys.stdout = self.original_stdout + self.wrapped_stdout = 0 + if not self.wrapped_stderr: + self.unwrap_excepthook() + # Deliver after releasing `lock`: this writes into the original + # stream and may log, see `_apply_capturing` + self._deliver_dropped(taken) def unwrap_stderr(self) -> None: """Undo one `wrap_stderr()` call, restoring at refcount zero. @@ -686,15 +1091,22 @@ def unwrap_stderr(self) -> None: excepthook is unwrapped once `stdout` is back to its original too. """ - if self.wrapped_stderr > 1: - self.wrapped_stderr -= 1 - else: - # Also reset our own reference so needs_clear() and - # update_capturing() don't act on a stale wrapper - self.stderr = sys.stderr = self.original_stderr - self.wrapped_stderr = 0 - if not self.wrapped_stdout: - self.unwrap_excepthook() + with self.lock: + if self.wrapped_stderr > 1: + self.wrapped_stderr -= 1 + return + + try: + taken = self._drop(self.stderr) + finally: + # Also reset our own reference so needs_clear() and + # update_capturing() don't act on a stale wrapper + self.stderr = sys.stderr = self.original_stderr + self.wrapped_stderr = 0 + if not self.wrapped_stdout: + self.unwrap_excepthook() + # Deliver after releasing `lock`, see `unwrap_stdout` + self._deliver_dropped(taken) def needs_clear(self) -> bool: # pragma: no cover """Return whether either wrapped stream has buffered output. @@ -714,34 +1126,13 @@ def needs_clear(self) -> bool: # pragma: no cover def flush(self) -> None: """Flush buffered captured output on both wrapped streams. - If writing the buffered text to a stream's target raises - `io.UnsupportedOperation` (as happens for some non-seekable - streams), that stream's redirection disables itself: - `wrapped_stdout`/`wrapped_stderr` is reset to 0 so this method - stops attempting to flush it on future calls, and a warning is - logged. `sys.stdout`/`sys.stderr` are left installed as-is: - only further flush attempts are skipped, not the wrapping - itself. + Native buffers are drained under `lock`. Custom drains and + writes to original streams run after releasing it. See + `_deliver` for what happens when a stream cannot take the text. """ - if self.wrapped_stdout and isinstance(self.stdout, WrappingIO): - try: - self.stdout._flush() - except io.UnsupportedOperation: - self.wrapped_stdout = 0 - logger.warning( - 'Disabling stdout redirection, %r is not seekable', - sys.stdout, - ) - - if self.wrapped_stderr and isinstance(self.stderr, WrappingIO): - try: - self.stderr._flush() - except io.UnsupportedOperation: # pragma: no cover - self.wrapped_stderr = 0 - logger.warning( - 'Disabling stderr redirection, %r is not seekable', - sys.stderr, - ) + with self.lock: + pending: list[_Pending] = self._take() + self._deliver(pending) def excepthook( self, diff --git a/progressbar/widgets.py b/progressbar/widgets.py index ed3cc40d..2bdf78bb 100644 --- a/progressbar/widgets.py +++ b/progressbar/widgets.py @@ -8,12 +8,14 @@ from __future__ import annotations +import _string # type: ignore[import-not-found] import abc import collections.abc import contextlib import datetime import functools import logging +import string import typing from typing import ClassVar @@ -27,6 +29,102 @@ logger = logging.getLogger(__name__) + +class _SafeFormatter(string.Formatter): + """`str.format` over widget data that refuses dunder attribute walks. + + Format templates may come from text the application did not write + (a `prefix`, a `Variable` format). A field such as + `{variables.__class__.__init__.__globals__}` would otherwise reach + module globals, so any attribute segment starting with `__` raises + `ValueError` before it is looked up. A single leading underscore is + allowed: `{variables._my_var}` is a documented way to reach a + variable, and a user object's own `_private` attribute is the + user's business. + """ + + #: Non-dunder attributes that also lead from an object stored as a + #: bar variable (a generator, coroutine, traceback or frame) to a + #: frame and its globals. + blocked_attributes: frozenset[str] = frozenset( + { + 'f_back', + 'f_builtins', + 'f_globals', + 'f_locals', + 'gi_frame', + 'cr_frame', + 'ag_frame', + 'tb_frame', + } + ) + + def get_field( + self, + field_name: str, + args: typing.Sequence[typing.Any], + kwargs: typing.Mapping[str, typing.Any], + ) -> tuple[typing.Any, str]: + self.validate_field(field_name) + return super().get_field(field_name, args, kwargs) + + def validate_field(self, field_name: str) -> None: + """Check every attribute before any value is looked up.""" + rest: typing.Iterable[tuple[bool, typing.Any]] + _first, rest = _string.formatter_field_name_split(field_name) + for is_attr, name in rest: + if is_attr and ( + str(name).startswith('__') or name in self.blocked_attributes + ): + raise ValueError( + f'attribute {name!r} is not allowed in a format template', + ) + + def validate(self, template: str, depth: int = 2) -> None: + """Check specialised templates before handing them to their owner.""" + if depth < 0: + raise ValueError('Max string recursion exceeded') + for _, field, spec, conversion in self.parse(template): + if field is not None: + self.validate_field(field) + if conversion is not None and conversion not in ( + 's', + 'r', + 'a', + ): + raise ValueError( + f'Unknown conversion specifier {conversion}' + ) + self.validate(spec or '', depth - 1) + + +_SAFE_FORMATTER: _SafeFormatter = _SafeFormatter() + + +def _safe_format( + template: object, + data: typing.Mapping[str, typing.Any], +) -> str: + """Render safe field paths while preserving specialised formatting. + + Application-owned format methods retain their escaping and result + types. Field paths in the underlying string or proxy text are + validated before delegation. Application code in format methods + and proxies remains trusted. + """ + if type(template) is str: + return _SAFE_FORMATTER.vformat(template, (), data) + formatter: typing.Any = getattr(template, 'format', None) + if not callable(formatter): + raise TypeError( + f'format template must be a str, got {type(template).__name__}', + ) + _SAFE_FORMATTER.validate( + str.__str__(template) if isinstance(template, str) else str(template) + ) + return typing.cast(str, formatter(**data)) + + MAX_DATE = datetime.date.max MAX_TIME = datetime.time.max MAX_DATETIME = datetime.datetime.max @@ -198,14 +296,14 @@ def __call__( format_ = self.get_format(progress, data, format) try: if self.new_style: - return format_.format(**data) + return _safe_format(format_, data) else: return format_ % data - except (TypeError, KeyError): + except (TypeError, KeyError, IndexError, ValueError): logger.exception( - 'Error while formatting %r with data: %r', + 'Error while formatting %r with data keys: %r', format_, - data, + list(data), ) raise @@ -2072,7 +2170,7 @@ def __call__( else: context['formatted_value'] = '-' * self.width - return self.format.format(**context) + return _safe_format(self.format, context) class DynamicMessage(Variable): diff --git a/pyproject.toml b/pyproject.toml index 5797b8ea..544e3d47 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -107,7 +107,7 @@ classifiers = [ ] requires-python = '>=3.10' -dependencies = ['python-utils >= 3.8.1'] +dependencies = ['python-utils >= 4.1.4'] [project.urls] bugs = 'https://github.com/wolph/python-progressbar/issues' @@ -133,6 +133,7 @@ docs = [ ] tests = [ 'dill>=0.3.6', + 'markupsafe>=2.0', 'freezegun>=0.3.11', 'pytest-cov>=2.6.1', 'pytest>=4.6.9', diff --git a/ruff.toml b/ruff.toml index 7a95ec7d..7b5dda6c 100644 --- a/ruff.toml +++ b/ruff.toml @@ -121,3 +121,7 @@ max-line-length = 79 [lint.flake8-pytest-style] mark-parentheses = true + +[lint.flake8-import-conventions.aliases] +# Preserve the established module spelling, including public annotations. +datetime = 'datetime' diff --git a/tests/api_surface_snapshot.json b/tests/api_surface_snapshot.json index b254d0e1..0d8e6674 100644 --- a/tests/api_surface_snapshot.json +++ b/tests/api_surface_snapshot.json @@ -481,7 +481,7 @@ "StringT": "type-alias", "T": "type-alias", "TracebackType": "re-export", - "WrappingIO": "class(target, capturing=?, listeners=?)", + "WrappingIO": "class(target, capturing=?, listeners=?, lock=?)", "annotations": "_Feature", "deltas_to_seconds": "callable(*deltas, default=?)", "epoch": "datetime", diff --git a/tests/test_data_transfer_bar.py b/tests/test_data_transfer_bar.py index e8f5c577..6b33b0c2 100644 --- a/tests/test_data_transfer_bar.py +++ b/tests/test_data_transfer_bar.py @@ -29,3 +29,23 @@ def test_file_transfer_speed_before_any_data() -> None: output = widget(bar, bar.data()) assert 's/' not in output bar.finish(dirty=True) + + +def test_data_size_below_one_byte_keeps_the_bare_unit() -> None: + # B28: python-utils before 4.1.4 scaled 0.0001 to (0.1024, -1), and + # the negative power picked the largest prefix ('Yi') off the end + # of the tuple. The dependency floor must rule that version out. + widget = progressbar.DataSize() + bar = progressbar.ProgressBar( + max_value=10, widgets=[widget], fd=io.StringIO(), term_width=60 + ) + bar.start() + rendered = widget(bar, {'value': 0.0001}) + bar.finish(dirty=True) + assert rendered.strip() == '0.0 B' + + +def test_format_unit_value_below_one_keeps_the_bare_unit() -> None: + from progressbar.widgets import format_unit_value + + assert format_unit_value(0.0001, unit='B', unit_scale=True) == '0.0 B' diff --git a/tests/test_deprecation_warnings.py b/tests/test_deprecation_warnings.py new file mode 100644 index 00000000..9dfcdb5d --- /dev/null +++ b/tests/test_deprecation_warnings.py @@ -0,0 +1,95 @@ +"""The deprecation warnings must point at the caller, not at progressbar. + +A warning attributed to `progressbar/bar.py` tells the user nothing about +which of their own lines used the deprecated name. Each test below records +the warning and checks that it lands in this file, on the line that used +`maxval`, `poll` or `currval`, whatever the depth of the `__init__` chain +in between. +""" + +from __future__ import annotations + +import sys +import typing +import warnings + +import progressbar + + +def _line_before() -> int: + """Return the line number of the statement above the calling line.""" + return sys._getframe(1).f_lineno - 1 + + +def _single_deprecation( + caught: list[warnings.WarningMessage], +) -> warnings.WarningMessage: + assert len(caught) == 1 + assert caught[0].category is DeprecationWarning + return caught[0] + + +class _BarWithOwnInit(progressbar.ProgressBar): + """A user subclass that adds one frame between the caller and the bar.""" + + super_call_line: int + + def __init__(self, **kwargs: typing.Any) -> None: + super().__init__(**kwargs) + self.super_call_line = _line_before() + + +def test_maxval_warning_points_at_caller() -> None: + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always', DeprecationWarning) + progressbar.ProgressBar(maxval=10) + expected_line: int = _line_before() + + warning: warnings.WarningMessage = _single_deprecation(caught) + assert warning.filename == __file__ + assert warning.lineno == expected_line + + +def test_poll_warning_points_at_caller() -> None: + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always', DeprecationWarning) + progressbar.ProgressBar(poll=1) + expected_line: int = _line_before() + + warning: warnings.WarningMessage = _single_deprecation(caught) + assert warning.filename == __file__ + assert warning.lineno == expected_line + + +def test_currval_warning_points_at_caller() -> None: + bar: progressbar.ProgressBar = progressbar.ProgressBar(max_value=10) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always', DeprecationWarning) + assert bar.currval == 0 + expected_line: int = _line_before() + + warning: warnings.WarningMessage = _single_deprecation(caught) + assert warning.filename == __file__ + assert warning.lineno == expected_line + + +def test_subclass_maxval_warning_points_at_caller() -> None: + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always', DeprecationWarning) + progressbar.DataTransferBar(maxval=10) + expected_line: int = _line_before() + + warning: warnings.WarningMessage = _single_deprecation(caught) + assert warning.filename == __file__ + assert warning.lineno == expected_line + + +def test_user_subclass_init_warning_points_at_super_call() -> None: + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always', DeprecationWarning) + bar: _BarWithOwnInit = _BarWithOwnInit(maxval=10) + + warning: warnings.WarningMessage = _single_deprecation(caught) + assert warning.filename == __file__ + assert warning.lineno == bar.super_call_line diff --git a/tests/test_env_detection.py b/tests/test_env_detection.py index 8a393a0f..dbfb4b32 100644 --- a/tests/test_env_detection.py +++ b/tests/test_env_detection.py @@ -10,6 +10,11 @@ from __future__ import annotations +import enum +import io +import os +import sys +import types import typing import pytest @@ -93,3 +98,32 @@ def test_is_terminal_plain_tty_without_ansi(clean_environment: None) -> None: def test_is_terminal_non_tty(clean_environment: None) -> None: fd = typing.cast(typing.IO[str], TtyFd(False)) assert env.is_terminal(fd) is False + + +def test_windows_console_mode_ignored_for_non_tty( + clean_environment: None, + monkeypatch: pytest.MonkeyPatch, +) -> None: + # The console-mode probe reads the process console, not `fd`: a + # redirected stream must not be reported as an ANSI terminal just + # because the console happens to have processed output enabled. + fake_windows: types.ModuleType = types.ModuleType('fake_windows') + + class FakeFlags(enum.IntFlag): + ENABLE_PROCESSED_OUTPUT = 1 + + def get_console_mode() -> int: # pragma: no cover - must not be called + return 1 + + fake_windows.WindowsConsoleModeFlags = FakeFlags # type: ignore[attr-defined] + fake_windows.get_console_mode = get_console_mode # type: ignore[attr-defined] + monkeypatch.setitem( + sys.modules, 'progressbar.terminal.os_specific.windows', fake_windows + ) + monkeypatch.setattr( + 'progressbar.terminal.os_specific.windows', + fake_windows, + raising=False, + ) + monkeypatch.setattr(os, 'name', 'nt') + assert env.is_ansi_terminal(io.StringIO()) is not True diff --git a/tests/test_format_security.py b/tests/test_format_security.py new file mode 100644 index 00000000..ca48af2c --- /dev/null +++ b/tests/test_format_security.py @@ -0,0 +1,374 @@ +"""Format templates must not walk private attributes or crash on desc.""" + +from __future__ import annotations + +import io + +import pytest + +import progressbar + + +def _render(**kwargs: object) -> str: + fd: io.StringIO = io.StringIO() + bar: progressbar.ProgressBar = progressbar.ProgressBar( + fd=fd, + max_value=10, + term_width=200, + **kwargs, # type: ignore[arg-type] + ) + bar.start() + bar.update(5) + bar.finish() + return fd.getvalue() + + +def test_desc_attribute_walk_renders_literally() -> None: + desc: str = ( + '{variables.__class__.__getattr__.__globals__[os].environ[HOME]}' + ) + output: str = _render(desc=desc) + assert desc + ': ' in output + + +def test_desc_with_braces_renders_literally() -> None: + output: str = _render(desc='report{2024}.txt') + assert 'report{2024}.txt: ' in output + + +def test_prefix_private_attribute_raises() -> None: + with pytest.raises(ValueError, match='__class__'): + _render(prefix='{variables.__class__}') + + +def test_prefix_value_template_still_renders() -> None: + output: str = _render(prefix='{value} ') + assert '10 ' in output + + +def test_prefix_variable_template_still_renders() -> None: + output: str = _render( + prefix='{variables.name} ', + variables={'name': 'spam'}, + ) + assert 'spam ' in output + + +def test_variable_format_private_attribute_raises() -> None: + widget: progressbar.Variable = progressbar.Variable( + 'name', + format='{formatted_value.__class__}', + ) + with pytest.raises(ValueError, match='__class__'): + _render(widgets=[widget], variables={'name': 'eggs'}) + + +def test_format_error_logs_keys_not_values( + caplog: pytest.LogCaptureFixture, +) -> None: + with pytest.raises(KeyError): + _render(prefix='{missing} ', variables={'secret': 'hunter2'}) + assert 'hunter2' not in caplog.text + + +@pytest.mark.parametrize('raw', ['', 'inf', 'nan', 'spam', '-1']) +def test_bad_minimum_update_interval_env_is_ignored( + monkeypatch: pytest.MonkeyPatch, + raw: str, +) -> None: + monkeypatch.setenv('PROGRESSBAR_MINIMUM_UPDATE_INTERVAL', raw) + from progressbar import bar as bar_module + + assert bar_module._env_minimum_update_interval() == 0.0 + output: str = _render() + assert output + + +def test_valid_minimum_update_interval_env_is_used( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv('PROGRESSBAR_MINIMUM_UPDATE_INTERVAL', '0.5') + from progressbar import bar as bar_module + + assert bar_module._env_minimum_update_interval() == 0.5 + + +# ---- round-3 take-backs ------------------------------------------------- + + +class _LazyStr: + """A translation proxy shape: not a str, but has format() and __str__.""" + + def __init__(self, text: str) -> None: + self.text: str = text + + def format(self, *args: object, **kwargs: object) -> str: + return self.text.format(*args, **kwargs) + + def __str__(self) -> str: + return self.text + + +def _label_data() -> tuple[progressbar.ProgressBar, dict[str, object]]: + bar: progressbar.ProgressBar = progressbar.ProgressBar( + fd=io.StringIO(), max_value=10, variables={'_x': 'v1', 'x': 1} + ) + bar.start() + bar.update(3, _x='v2') + return bar, dict(bar.data()) + + +def test_lazy_template_object_renders() -> None: + bar, data = _label_data() + label = progressbar.FormatLabel( + _LazyStr('{value} of {max_value}'), # type: ignore[arg-type] + new_style=True, + ) + assert label(bar, data) == '3 of 10' + + +def test_single_underscore_variable_renders() -> None: + bar, data = _label_data() + label = progressbar.FormatLabel('{variables._x}', new_style=True) + assert label(bar, data) == 'v2' + + +def test_dunder_attribute_still_refused() -> None: + bar, data = _label_data() + label = progressbar.FormatLabel('{variables.x.__class__}', new_style=True) + with pytest.raises(ValueError, match='__class__'): + label(bar, data) + + +def test_prefix_reassigned_after_desc_is_a_template() -> None: + fd: io.StringIO = io.StringIO() + bar: progressbar.ProgressBar = progressbar.ProgressBar( + fd=fd, max_value=10, desc='d', widgets=[progressbar.SimpleProgress()] + ) + bar.prefix = 'late {value} ' + bar.start() + bar.update(4, force=True) + assert 'late 4 4 of 10' in fd.getvalue() + assert '{value}' not in fd.getvalue() + + +def test_format_error_with_non_str_keys_raises_the_template_error() -> None: + bar, data = _label_data() + data[0] = 'zero' # type: ignore[index] + label = progressbar.FormatLabel('{nope}', new_style=True) + with pytest.raises(KeyError, match='nope'): + label(bar, data) + + +def test_bad_minimum_update_interval_env_logs_a_warning( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + import logging + + from progressbar import bar as bar_module + + bar_module._reset_bad_interval_warnings() + monkeypatch.setenv('PROGRESSBAR_MINIMUM_UPDATE_INTERVAL', 'spam') + with caplog.at_level(logging.WARNING, logger='progressbar.bar'): + progressbar.ProgressBar(fd=io.StringIO(), max_value=10) + assert any( + 'PROGRESSBAR_MINIMUM_UPDATE_INTERVAL' in record.getMessage() + and 'spam' in record.getMessage() + for record in caplog.records + ) + + +def test_non_text_template_without_format_raises() -> None: + bar, data = _label_data() + + class NotCallableFormat: + format = 'text' + + for template in (None, ['{value}'], 42, NotCallableFormat()): + label = progressbar.FormatLabel(template, new_style=True) # type: ignore[arg-type] + with pytest.raises(TypeError): + label(bar, data) + + +def test_bad_minimum_update_interval_env_warns_once_per_value( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + import logging + + from progressbar import bar as bar_module + + bar_module._reset_bad_interval_warnings() + monkeypatch.setenv('PROGRESSBAR_MINIMUM_UPDATE_INTERVAL', 'eggs') + with caplog.at_level(logging.WARNING, logger='progressbar.bar'): + for _ in range(3): + progressbar.ProgressBar(fd=io.StringIO(), max_value=10) + records = [r for r in caplog.records if 'eggs' in r.getMessage()] + assert len(records) == 1 + + +def test_bad_interval_warning_is_once_per_value_beyond_sixteen_and_threadsafe( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + import logging + import threading + + from progressbar import bar as bar_module + + bar_module._reset_bad_interval_warnings() + with caplog.at_level(logging.WARNING, logger='progressbar.bar'): + for i in range(17): + bar_module._warn_bad_minimum_update_interval(f'bad-{i}') + bar_module._warn_bad_minimum_update_interval('bad-0') + barrier: threading.Barrier = threading.Barrier(4) + + def warn() -> None: + barrier.wait() + bar_module._warn_bad_minimum_update_interval('concurrent') + + threads: list[threading.Thread] = [ + threading.Thread(target=warn) for _ in range(4) + ] + for thread in threads: + thread.start() + for thread in threads: + thread.join() + messages: list[str] = [r.getMessage() for r in caplog.records] + assert sum("'bad-0'" in m for m in messages) == 1 + assert sum("'concurrent'" in m for m in messages) == 1 + assert len(messages) == 18 + + +def test_desc_prefix_is_a_format_label_that_renders_literally() -> None: + fd: io.StringIO = io.StringIO() + bar: progressbar.ProgressBar = progressbar.ProgressBar( + fd=fd, + max_value=10, + desc='{files}', + widgets=[progressbar.SimpleProgress()], + ) + assert bar.prefix == '{files}: ' # read back unescaped, as in 4.6.0 + bar.start() + assert isinstance(bar.widgets[0], progressbar.FormatLabel) # as in 4.6.0 + bar.update(4, force=True) + assert '{files}: 4 of 10' in fd.getvalue() + + +def test_desc_with_braces_renders_literally_on_the_fast_path() -> None: + fd: io.StringIO = io.StringIO() + bar: progressbar.ProgressBar = progressbar.ProgressBar( + fd=fd, max_value=10, desc='{files}' + ) + bar.start() + bar.update(4, force=True) + assert '{files}: ' in fd.getvalue() + assert '{{files}}' not in fd.getvalue() + fast: progressbar.FastProgressBar = progressbar.FastProgressBar( + fd=io.StringIO(), max_value=10, desc='{files}' + ) + assert fast.prefix == '{files}: ' + from progressbar import fast as fast_module + + line: str = fast_module._pure_format_fast_line(fast) + assert line.startswith('{files}: ') + + +def test_frame_walks_are_refused_like_dunders() -> None: + import sys + import types + + bar, data = _label_data() + frame: types.FrameType = sys._getframe() + data['variables'] = progressbar.utils.AttributeDict(frame=frame) + for template in ( + '{variables.frame.f_globals}', + '{variables.frame.f_locals}', + '{variables.frame.f_back}', + ): + label = progressbar.FormatLabel(template, new_style=True) + with pytest.raises(ValueError, match='not allowed'): + label(bar, data) + + +def test_specialised_template_preserves_markup_escaping_and_type() -> None: + from markupsafe import Markup + + bar, data = _label_data() + data['value'] = '' + label: progressbar.FormatLabel = progressbar.FormatLabel( + Markup('{value}'), new_style=True + ) + result: str = label(bar, data) + assert result == '<tag>' + assert isinstance(result, Markup) + + +@pytest.mark.parametrize( + 'template', + [ + '{value.__class__!s}', + '{value:{width.__class__}}', + '{value:{width.f_globals}}', + '{value!z}', + '{value:{width:{precision}}}', + ], +) +def test_specialised_template_validates_before_delegation( + template: str, +) -> None: + calls: list[str] = [] + + class Template(str): + def format(self, *args: object, **kwargs: object) -> str: + calls.append(self) + return 'specialised' + + bar, data = _label_data() + label: progressbar.FormatLabel = progressbar.FormatLabel( + Template(template), new_style=True + ) + with pytest.raises(ValueError): + label(bar, data) + assert calls == [] + + +def test_specialised_proxy_preserves_format_method() -> None: + class Template(_LazyStr): + def format(self, *args: object, **kwargs: object) -> str: + return 'specialised:' + super().format(*args, **kwargs) + + bar, data = _label_data() + label: progressbar.FormatLabel = progressbar.FormatLabel( + Template('{value}'), new_style=True + ) # type: ignore[arg-type] + assert label(bar, data) == 'specialised:3' + + +def test_specialised_str_cannot_hide_field_paths_in_str_override() -> None: + calls: list[str] = [] + + class Template(str): + def __str__(self) -> str: + return '{value}' + + def format(self, *args: object, **kwargs: object) -> str: + calls.append('called') + return super().format(*args, **kwargs) + + bar, data = _label_data() + label: progressbar.FormatLabel = progressbar.FormatLabel( + Template('{value.__class__}'), new_style=True + ) + with pytest.raises(ValueError, match='__class__'): + label(bar, data) + assert calls == [] + + +def test_specialised_template_allows_literal_braces_and_nested_width() -> None: + from markupsafe import Markup + + bar, data = _label_data() + data['width'] = 3 + label: progressbar.FormatLabel = progressbar.FormatLabel( + Markup('{{literal}} {value:0{width}d}'), new_style=True + ) + assert label(bar, data) == '{literal} 003' diff --git a/tests/test_multibar.py b/tests/test_multibar.py index 9b6d66fd..b8ea8217 100644 --- a/tests/test_multibar.py +++ b/tests/test_multibar.py @@ -577,3 +577,628 @@ def test_multibar_concurrent_mutation() -> None: assert not errors assert not multibar._thread or not multibar._thread.is_alive() + + +def test_render_of_a_bar_removed_mid_frame_does_not_leak() -> None: + # The render thread snapshots the bars, then a worker deletes one + # (task_finished); the finished-bar path must not stamp it back + # into _finished_at after __delitem__ popped it. + multibar = progressbar.MultiBar(fd=io.StringIO()) + bar_ = multibar['gone'] + bar_.start() + bar_.finish() + del multibar['gone'] + lines = list( + multibar._render_finished_bar( + bar_, time.monotonic(), None, lambda **kw: None + ) + ) + assert lines == [] + assert bar_ not in multibar._finished_at + + +@pytest.mark.parametrize('mutation', ['assign', 'update', 'ior']) +def test_expiry_keeps_concurrent_replacement(mutation: str) -> None: + import inspect + import sys + import types + import typing + + multibar: progressbar.MultiBar = progressbar.MultiBar( + fd=io.StringIO(), remove_finished=1 + ) + old: progressbar.ProgressBar = multibar['task'] + replacement: progressbar.ProgressBar = progressbar.ProgressBar( + fd=io.StringIO() + ) + multibar._finished_at[old] = 0 + multibar._labeled.add(old) + reached: threading.Event = threading.Event() + resume: threading.Event = threading.Event() + errors: list[BaseException] = [] + lines, first = inspect.getsourcelines(multibar._render_finished_bar) + pause_line: int = first + next( + i for i, line in enumerate(lines) if 'self.remove_finished' in line + ) + + def trace( + frame: types.FrameType, event: str, arg: typing.Any + ) -> typing.Any: + if ( + event == 'line' + and frame.f_code is multibar._render_finished_bar.__func__.__code__ + and frame.f_lineno == pause_line + ): + reached.set() + assert resume.wait(3) + return trace + + def render() -> None: + sys.settrace(trace) + try: + list( + multibar._render_finished_bar(old, 10, 9, lambda **kwargs: '') + ) + except BaseException as error: + errors.append(error) + finally: + sys.settrace(None) + + thread: threading.Thread = threading.Thread(target=render) + thread.start() + try: + assert reached.wait(3) + if mutation == 'assign': + multibar['task'] = replacement + elif mutation == 'update': + multibar.update(task=replacement) + else: + multibar |= {'task': replacement} + finally: + resume.set() + thread.join(3) + assert not thread.is_alive() + assert errors == [] + assert multibar.get('task') is replacement + assert old not in multibar._finished_at + assert old not in multibar._labeled + assert replacement.label == 'task' + + +@pytest.mark.parametrize('removal', ['pop', 'popitem', 'clear', 'del']) +def test_membership_removal_cleans_tracking(removal: str) -> None: + multibar: progressbar.MultiBar = progressbar.MultiBar(fd=io.StringIO()) + progress: progressbar.ProgressBar = multibar['task'] + multibar._finished_at[progress] = 0 + multibar._labeled.add(progress) + if removal == 'pop': + assert multibar.pop('task') is progress + elif removal == 'popitem': + assert multibar.popitem() == ('task', progress) + elif removal == 'clear': + multibar.clear() + else: + del multibar['task'] + assert not multibar + assert not multibar._finished_at + assert not multibar._labeled + + +def test_membership_dict_defaults_and_update_iterable() -> None: + multibar: progressbar.MultiBar = progressbar.MultiBar(fd=io.StringIO()) + progress: progressbar.ProgressBar = progressbar.ProgressBar( + fd=io.StringIO() + ) + assert multibar.setdefault('task', progress) is progress + assert multibar.setdefault('task') is progress + assert multibar.setdefault('none') is None + assert multibar.pop('none') is None + assert multibar.pop('missing', 42) == 42 + with pytest.raises(KeyError): + multibar.pop('missing') + with pytest.raises(TypeError): + multibar.pop('missing', 1, 2) + multibar.clear() + with pytest.raises(KeyError): + multibar.popitem() + multibar.update([('task', progress)]) + assert multibar['task'] is progress + + +def test_membership_guard_is_released_before_finished_update() -> None: + multibar: progressbar.MultiBar = progressbar.MultiBar(fd=io.StringIO()) + progress: progressbar.ProgressBar = multibar['task'] + unlocked: list[bool] = [] + + def update(**kwargs: object) -> str: + def observer() -> None: + acquired: bool = multibar._membership_lock.acquire(timeout=1) + unlocked.append(acquired) + if acquired: + multibar._membership_lock.release() + + thread: threading.Thread = threading.Thread(target=observer) + thread.start() + thread.join(2) + assert not thread.is_alive() + return '' + + list(multibar._render_finished_bar(progress, 10, None, update)) + assert unlocked == [True] + + +@pytest.mark.parametrize( + 'operation', ['replace', 'pop', 'popitem', 'clear', 'finish'] +) +def test_membership_tracking_never_calls_bar_hash(operation: str) -> None: + class HashBar(progressbar.ProgressBar): + armed: bool = False + + def __hash__(self) -> int: + if self.armed: + raise AssertionError('bar hash callback') + return object.__hash__(self) + + multibar: progressbar.MultiBar = progressbar.MultiBar(fd=io.StringIO()) + progress: HashBar = HashBar(fd=io.StringIO()) + multibar['task'] = progress + multibar._finished_at[progress] = 0 + multibar._labeled.add(progress) + progress.armed = True + if operation == 'replace': + multibar['task'] = progressbar.ProgressBar(fd=io.StringIO()) + elif operation == 'pop': + multibar.pop('task') + elif operation == 'popitem': + multibar.popitem() + elif operation == 'clear': + multibar.clear() + else: + list( + multibar._render_finished_bar( + progress, 10, None, lambda **kwargs: '' + ) + ) + progress.armed = False + + +def test_multibar_membership_lock_resets_after_fork() -> None: + import os + import subprocess + import sys + import textwrap + + if not hasattr(os, 'fork'): + pytest.skip('fork requires POSIX') + code: str = textwrap.dedent(""" + import io + import os + import signal + import threading + import progressbar + + multibar = progressbar.MultiBar(fd=io.StringIO()) + held = threading.Event() + release = threading.Event() + def holder(): + with multibar._membership_lock: + held.set() + release.wait(5) + thread = threading.Thread(target=holder, daemon=True) + thread.start() + assert held.wait(2) + pid = os.fork() + if pid == 0: + signal.alarm(2) + multibar['child'] = progressbar.ProgressBar(fd=io.StringIO()) + os._exit(0) + _, status = os.waitpid(pid, 0) + release.set() + thread.join(2) + assert os.waitstatus_to_exitcode(status) == 0 + """) + result: subprocess.CompletedProcess[str] = subprocess.run( + [sys.executable, '-c', code], + capture_output=True, + text=True, + timeout=15, + ) + assert result.returncode == 0, result.stderr + + +def test_tracking_distinguishes_equal_bars_by_identity() -> None: + class EqualBar(progressbar.ProgressBar): + def __hash__(self) -> int: + raise AssertionError('tracking called hash') + + def __eq__(self, other: object) -> bool: + raise AssertionError('tracking called equality') + + multibar: progressbar.MultiBar = progressbar.MultiBar(fd=io.StringIO()) + first: EqualBar = EqualBar(fd=io.StringIO()) + second: EqualBar = EqualBar(fd=io.StringIO()) + multibar['first'] = first + multibar['second'] = second + multibar._finished_at[first] = 1 + multibar._finished_at[second] = 2 + multibar._labeled.add(first) + multibar._labeled.add(second) + assert len(multibar._finished_at) == len(multibar._labeled) == 2 + assert multibar._finished_at[first] == 1 + assert multibar._finished_at[second] == 2 + assert next(iter(multibar._finished_at)) is first + assert list(multibar._labeled)[1] is second + del multibar['first'] + assert first not in multibar._finished_at + assert first not in multibar._labeled + assert second in multibar._finished_at + assert second in multibar._labeled + multibar.clear() + assert not multibar._finished_at + assert not multibar._labeled + + +class _IdentityTracked: + def __init__(self) -> None: + self.owner: object = None + + def __hash__(self) -> int: + raise AssertionError('hash callback') + + def __eq__(self, other: object) -> bool: + raise AssertionError('equality callback') + + +@pytest.mark.parametrize('method', ['deepcopy', 'pickle']) +@pytest.mark.parametrize('kind', ['mapping', 'set']) +def test_identity_tracking_copy_rebuilds_ids_and_cycles( + method: str, kind: str +) -> None: + import copy + import pickle + import typing + + from progressbar import _identity + + tracked: _IdentityTracked = _IdentityTracked() + collection: typing.Any + if kind == 'mapping': + collection = _identity.IdentityMap() + collection[tracked] = 'value' + else: + collection = _identity.IdentitySet() + collection.add(tracked) + tracked.owner = collection + restored: typing.Any = ( + copy.deepcopy(collection) + if method == 'deepcopy' + else pickle.loads(pickle.dumps(collection)) + ) + copied: _IdentityTracked = next(iter(restored)) + assert copied is not tracked + assert copied.owner is restored + assert copied in restored + assert tracked not in restored + if kind == 'mapping': + assert restored[copied] == 'value' + del restored[copied] + else: + restored.discard(copied) + assert not restored + + +@pytest.mark.parametrize('operation', ['replace', 'clear', 'pop', 'popitem']) +def test_removed_bar_finalizer_runs_outside_membership_guard( + operation: str, +) -> None: + observed: list[bool] = [] + multibar: progressbar.MultiBar = progressbar.MultiBar(fd=io.StringIO()) + + class FinalBar(progressbar.ProgressBar): + def __del__(self) -> None: + def observer() -> None: + acquired: bool = multibar._membership_lock.acquire(timeout=1) + observed.append(acquired) + if acquired: + multibar._membership_lock.release() + + thread: threading.Thread = threading.Thread(target=observer) + thread.start() + thread.join(2) + super().__del__() + + multibar['task'] = FinalBar(fd=io.StringIO()) + multibar._finished_at[multibar['task']] = 0 + multibar._labeled.add(multibar['task']) + if operation == 'replace': + multibar['task'] = progressbar.ProgressBar(fd=io.StringIO()) + elif operation == 'clear': + multibar.clear() + elif operation == 'pop': + multibar.pop('task') + else: + multibar.popitem() + assert observed == [True] + + +def test_render_does_not_relabel_replaced_bar() -> None: + import inspect + import sys + import types + import typing + + from progressbar import multi + + multibar: progressbar.MultiBar = progressbar.MultiBar(fd=io.StringIO()) + old: progressbar.ProgressBar = progressbar.ProgressBar( + fd=io.StringIO(), widgets=['old'], max_value=1 + ) + replacement: progressbar.ProgressBar = progressbar.ProgressBar( + fd=io.StringIO() + ) + multibar['task'] = old + old.start() + old.finish() + lines, first = inspect.getsourcelines(multi.MultiBar._render_bar) + pause_line: int = first + next( + i for i, line in enumerate(lines) if 'self._label_bar(bar_' in line + ) + reached: threading.Event = threading.Event() + resume: threading.Event = threading.Event() + errors: list[BaseException] = [] + + def trace( + frame: types.FrameType, event: str, arg: typing.Any + ) -> typing.Any: + if ( + event == 'line' + and frame.f_code.co_filename == multi.__file__ + and frame.f_lineno == pause_line + ): + reached.set() + assert resume.wait(2) + return trace + + def render() -> None: + sys.settrace(trace) + try: + list(multibar._render_bar(old, now=10, expired=None)) + except BaseException as error: + errors.append(error) + finally: + sys.settrace(None) + + thread: threading.Thread = threading.Thread(target=render) + thread.start() + try: + assert reached.wait(2) + multibar['task'] = replacement + finally: + resume.set() + thread.join(2) + assert not thread.is_alive() + assert errors == [] + assert multibar.get('task') is replacement + assert old not in multibar._finished_at + assert old not in multibar._labeled + + +def test_expiry_rechecks_membership_after_custom_interval_callback() -> None: + multibar: progressbar.MultiBar = progressbar.MultiBar(fd=io.StringIO()) + old: progressbar.ProgressBar = multibar['task'] + replacement: progressbar.ProgressBar = progressbar.ProgressBar( + fd=io.StringIO() + ) + multibar._finished_at[old] = 0 + + class Interval(float): + def __bool__(self) -> bool: + multibar['task'] = replacement + return True + + multibar.remove_finished = Interval(1) + assert ( + list(multibar._render_finished_bar(old, 10, 9, lambda **kwargs: '')) + == [] + ) + assert multibar.get('task') is replacement + + +def test_label_reservation_skips_removed_bars_and_disabled_labels() -> None: + multibar: progressbar.MultiBar = progressbar.MultiBar( + fd=io.StringIO(), prepend_label=False, append_label=False + ) + progress: progressbar.ProgressBar = progressbar.ProgressBar( + fd=io.StringIO(), widgets=['value'] + ) + multibar['task'] = progress + assert multibar._label_bar(progress, require_member=True) + assert progress.widgets == ['value'] + assert progress not in multibar._labeled + del multibar['task'] + assert not multibar._label_bar(progress, require_member=True) + assert progress not in multibar._labeled + + +def _observe_membership_lock( + multibar: progressbar.MultiBar, observed: list[bool] +) -> None: + def acquire() -> None: + acquired: bool = multibar._membership_lock.acquire(timeout=0.1) + observed.append(acquired) + if acquired: + multibar._membership_lock.release() + + thread: threading.Thread = threading.Thread(target=acquire) + thread.start() + thread.join(2) + assert not thread.is_alive() + + +@pytest.mark.parametrize( + 'operation', ['assign', 'lookup', 'pop', 'setdefault', 'label', 'expire'] +) +@pytest.mark.parametrize('callback', ['hash', 'equal']) +def test_custom_label_callbacks_run_outside_membership_guard( + operation: str, callback: str +) -> None: + multibar: progressbar.MultiBar = progressbar.MultiBar( + fd=io.StringIO(), remove_finished=1 + ) + observed: list[bool] = [] + armed: bool = False + + class Label(str): + def __hash__(self) -> int: + if armed and callback == 'hash': + _observe_membership_lock(multibar, observed) + return str.__hash__(self) + + def __eq__(self, other: object) -> bool: + if armed and callback == 'equal': + _observe_membership_lock(multibar, observed) + return str.__eq__(self, other) + + label: Label = Label('task') + progress: progressbar.ProgressBar = progressbar.ProgressBar( + fd=io.StringIO(), widgets=['x'] + ) + multibar[label] = progress + assert next(iter(multibar)) is label + # Exact-str queries still call equality on the retained custom key. + key: str = label if callback == 'hash' else 'task' + armed = True + if operation == 'assign': + multibar[key] = progressbar.ProgressBar(fd=io.StringIO()) + assert next(iter(multibar)) is label + elif operation == 'lookup': + assert multibar[key] is progress + elif operation == 'pop': + assert multibar.pop(key) is progress + elif operation == 'setdefault': + assert multibar.setdefault(key) is progress + elif operation == 'label': + progress.label = key + multibar._label_bar(progress, require_member=True) + else: + progress.label = key + multibar._finished_at[progress] = 0 + list( + multibar._render_finished_bar(progress, 10, 9, lambda **kwargs: '') + ) + assert observed and all(observed) + + +def test_waiting_native_lookup_rechecks_custom_mode() -> None: + multibar: progressbar.MultiBar = progressbar.MultiBar(fd=io.StringIO()) + observed: list[bool] = [] + started: threading.Event = threading.Event() + errors: list[BaseException] = [] + armed: bool = False + + class Label(str): + __hash__ = str.__hash__ + + def __eq__(self, other: object) -> bool: + if armed: + + def acquire() -> None: + acquired: bool = multibar._membership_lock.acquire( + timeout=0.2 + ) + observed.append(acquired) + if acquired: + multibar._membership_lock.release() + + observer: threading.Thread = threading.Thread(target=acquire) + observer.start() + observer.join(2) + return str.__eq__(self, other) + + progress: progressbar.ProgressBar = progressbar.ProgressBar( + fd=io.StringIO() + ) + + def lookup() -> None: + started.set() + try: + assert multibar['task'] is progress + except BaseException as error: + errors.append(error) + + with multibar._membership_lock: + thread: threading.Thread = threading.Thread(target=lookup) + thread.start() + assert started.wait(2) + multibar[Label('task')] = progress + armed = True + thread.join(2) + assert not thread.is_alive() + assert errors == [] + assert observed and all(observed) + + +def test_clear_does_not_reenable_native_mode_during_custom_insert() -> None: + multibar: progressbar.MultiBar = progressbar.MultiBar(fd=io.StringIO()) + hashing: threading.Event = threading.Event() + resume: threading.Event = threading.Event() + observed: list[bool] = [] + errors: list[BaseException] = [] + armed: bool = False + + class Label(str): + def __hash__(self) -> int: + if ( + threading.current_thread().name == 'custom-insert' + and not hashing.is_set() + ): + hashing.set() + assert resume.wait(3) + return str.__hash__(self) + + def __eq__(self, other: object) -> bool: + if armed: + + def acquire() -> None: + acquired: bool = multibar._membership_lock.acquire( + timeout=0.2 + ) + observed.append(acquired) + if acquired: + multibar._membership_lock.release() + + observer: threading.Thread = threading.Thread(target=acquire) + observer.start() + observer.join(2) + return str.__eq__(self, other) + + label: Label = Label('task') + progress: progressbar.ProgressBar = progressbar.ProgressBar( + fd=io.StringIO() + ) + + def insert() -> None: + try: + multibar[label] = progress + except BaseException as error: + errors.append(error) + + thread: threading.Thread = threading.Thread( + target=insert, name='custom-insert' + ) + thread.start() + try: + assert hashing.wait(2) + # On the old code this lock remains held by the custom hash callback. + acquired: bool = multibar._membership_lock.acquire(timeout=0.2) + assert acquired + multibar._membership_lock.release() + multibar.clear() + finally: + resume.set() + thread.join(3) + assert errors == [] + armed = True + assert multibar['task'] is progress + assert next(iter(multibar)) is label + assert observed and all(observed) diff --git a/tests/test_parallel_display_security.py b/tests/test_parallel_display_security.py new file mode 100644 index 00000000..fd107a59 --- /dev/null +++ b/tests/test_parallel_display_security.py @@ -0,0 +1,78 @@ +"""Security regressions for the ``bar='multi'`` parallel display.""" + +from __future__ import annotations + +import io +import time +import typing + +import pytest + +import progressbar +from progressbar.utils import streams + +_HOSTILE_ITEM: str = 'a\x1b]0;x\x07b\x1bcc\x9b31md\re\nf' +_FORBIDDEN: tuple[str, ...] = ('\x1b]0;x\x07', '\x1bc', '\x9b', '\r', '\n') + + +def _identity(item: typing.Any) -> typing.Any: + return item + + +def _slow_identity(item: typing.Any) -> typing.Any: + # Keep the task running long enough for the render thread to draw it. + time.sleep(0.3) + return item + + +@pytest.mark.no_freezegun +def test_multi_labels_strip_control_sequences() -> None: + fd: io.StringIO = io.StringIO() + result: list[typing.Any] = list( + progressbar.map( + _slow_identity, + [_HOSTILE_ITEM], + bar='multi', + fd=fd, + poll_interval=0.05, + ) + ) + assert result == [_HOSTILE_ITEM] + output: str = fd.getvalue() + for sequence in ('\x1b]0;x\x07', '\x1bc', '\x9b'): + assert sequence not in output + assert '1: ab' in output + # The raw label's CR and LF must not reach the output inside it. + assert 'b\rc' not in output + assert 'd\re' not in output + assert 'e\nf' not in output + + +def test_display_label_removes_every_control() -> None: + from progressbar._parallel import _display + + cleaned: str = _display._display_label(_HOSTILE_ITEM + '\t\x7f\x1b[31m') + for sequence in (*_FORBIDDEN, '\t', '\x7f', '\x1b'): + assert sequence not in cleaned + + +def test_multi_map_restores_stream_state() -> None: + listeners_before: int = len(streams.listeners) + capturing_before: int = streams.capturing + result: list[int] = list( + progressbar.map(_identity, range(5), bar='multi', fd=io.StringIO()) + ) + assert result == list(range(5)) + assert len(streams.listeners) == listeners_before + assert streams.capturing == capturing_before + + +def test_task_started_accepts_a_non_str_label() -> None: + from progressbar._parallel import _display + + display = _display.MultiDisplay( + total=1, poll_interval=0.05, bar_kwargs={'fd': io.StringIO()} + ) + display.task_started(1, 123) # type: ignore[arg-type] + assert display._keys[1] == '1: 123' + display.task_finished(1, ok=True) diff --git a/tests/test_parallel_imap.py b/tests/test_parallel_imap.py index c020e372..5aff254c 100644 --- a/tests/test_parallel_imap.py +++ b/tests/test_parallel_imap.py @@ -74,14 +74,22 @@ def test_on_error_raise_raises_at_iteration(self) -> None: class TestImapUnordered: @pytest.mark.no_freezegun def test_yields_pairs_in_completion_order(self) -> None: - pairs: list[tuple[int, int]] = list( - _sync.imap_unordered( - _sleep_inverse, range(5), workers=5, bar=False - ) - ) + # Deterministic ordering: only item 4 can finish until the gate + # opens, so timing on a loaded machine cannot reorder the result + gate: threading.Event = threading.Event() + + def gated(value: int) -> int: + if value != 4: + assert gate.wait(10) + return value + + iterator = _sync.imap_unordered(gated, range(5), workers=5, bar=False) + first: tuple[int, int] = next(iterator) + gate.set() + pairs: list[tuple[int, int]] = [first, *iterator] assert sorted(pairs) == [(value, value) for value in range(5)] - # The fastest item (highest input) completes and is seen first. - assert pairs[0] == (4, 4) + # The only item that could complete is seen first. + assert first == (4, 4) def test_multi_iterable_pairs_use_args_tuple(self) -> None: pairs = list( diff --git a/tests/test_parallel_shell.py b/tests/test_parallel_shell.py index 100f6626..86b272b6 100644 --- a/tests/test_parallel_shell.py +++ b/tests/test_parallel_shell.py @@ -84,8 +84,35 @@ def test_shell_string_appends_quoted(self) -> None: command = _shell.build_argv('gzip -k', 'a file.txt', shell=True) assert command == "gzip -k 'a file.txt'" + def test_item_placeholder_text_is_not_reexpanded(self) -> None: + # One substitution pass: '{item}' inside the item stays literal. + argv: list[str] | str = _shell.build_argv( + 'echo {}', 'a{item}b', shell=False + ) + assert argv == ['echo', 'a{item}b'] + + def test_shell_item_placeholder_text_is_not_reexpanded(self) -> None: + command: list[str] | str = _shell.build_argv( + 'echo {item}', 'x{}y', shell=True + ) + assert command == 'echo x{}y' + + @pytest.mark.parametrize( + 'command', + [['echo', '{}'], lambda item: ['echo', str(item)]], + ) + def test_shell_rejects_non_str_template( + self, command: _shell.CommandT + ) -> None: + with pytest.raises(TypeError, match='str command template'): + _shell.build_argv(command, 'x', shell=True) + class TestRun: + def test_shell_with_list_template_raises(self) -> None: + with pytest.raises(TypeError, match='str command template'): + _shell.run(['echo', '{}'], ['x'], shell=True) + @pytest.mark.no_freezegun def test_runs_commands_and_returns_completed_processes(self) -> None: results: list[subprocess.CompletedProcess[str]] = _shell.run( diff --git a/tests/test_progressbar.py b/tests/test_progressbar.py index 04fe678b..c46a451e 100644 --- a/tests/test_progressbar.py +++ b/tests/test_progressbar.py @@ -1,6 +1,7 @@ import contextlib import gc import io +import logging import os import signal import sys @@ -344,47 +345,143 @@ def fail_finish(self): @pytest.mark.skipif(os.name == 'nt', reason='SIGWINCH is POSIX-only') -def test_sigwinch_restored_with_overlapping_bars() -> None: +def test_sigwinch_restored_with_overlapping_bars( + isolated_resize_registry, +) -> None: # Regression: A5 - with two live bars, finishing them in creation - # order left a dangling handler installed. + # order left a dangling handler installed. The fixture starts from + # SIG_IGN, so "still installed" and "restored" can be told apart. + bar1 = progressbar.ProgressBar(max_value=5, fd=io.StringIO()) + bar1.start() + bar2 = progressbar.ProgressBar(max_value=5, fd=io.StringIO()) + bar2.start() + + # The first bar installs the shared handler + assert signal.getsignal(signal.SIGWINCH) is not signal.SIG_IGN + + # A resize signal is dispatched to all live bars + signal.raise_signal(signal.SIGWINCH) + assert isinstance(bar1.term_width, int) + assert isinstance(bar2.term_width, int) + + bar1.update(5) + bar1.finish() + # The handler must stay installed while bar2 is still live + assert signal.getsignal(signal.SIGWINCH) is not signal.SIG_IGN + + bar2.update(5) + bar2.finish() + # The last bar to finish restores the previous handler + assert signal.getsignal(signal.SIGWINCH) is signal.SIG_IGN + + +@pytest.fixture +def isolated_resize_registry(): + """Give a test an empty `_ResizeRegistry` and the handler it found. + + The registry is process-global, so bars left registered by other + tests (and a handler they left installed) would otherwise leak into + the assertions. Everything is put back afterwards. + """ import progressbar.bar as bar_module + registry = bar_module._ResizeRegistry saved_handler = signal.getsignal(signal.SIGWINCH) - # Isolate the global registry so the assertions don't depend on bars - # left registered (and a handler left installed) by other tests - saved_bars = list(bar_module._ResizeRegistry.bars) - saved_prev = bar_module._ResizeRegistry.previous_handler - bar_module._ResizeRegistry.bars.clear() - bar_module._ResizeRegistry.previous_handler = None - - # Start from a known sentinel handler so we can tell apart "still - # installed" from "restored" without depending on global state + saved_bars = list(registry.bars) + saved_prev = registry.previous_handler + registry.bars.clear() + registry.previous_handler = None signal.signal(signal.SIGWINCH, signal.SIG_IGN) try: - bar1 = progressbar.ProgressBar(max_value=5, fd=io.StringIO()) - bar1.start() - bar2 = progressbar.ProgressBar(max_value=5, fd=io.StringIO()) - bar2.start() - - # The first bar installs the shared handler - assert signal.getsignal(signal.SIGWINCH) is not signal.SIG_IGN - - # A resize signal is dispatched to all live bars - signal.raise_signal(signal.SIGWINCH) - assert isinstance(bar1.term_width, int) - assert isinstance(bar2.term_width, int) - - bar1.update(5) - bar1.finish() - # The handler must stay installed while bar2 is still live - assert signal.getsignal(signal.SIGWINCH) is not signal.SIG_IGN - - bar2.update(5) - bar2.finish() - # The last bar to finish restores the previous handler - assert signal.getsignal(signal.SIGWINCH) is signal.SIG_IGN + yield registry finally: + registry.bars.clear() for restored_bar in saved_bars: - bar_module._ResizeRegistry.bars.add(restored_bar) - bar_module._ResizeRegistry.previous_handler = saved_prev + registry.bars.add(restored_bar) + registry.previous_handler = saved_prev signal.signal(signal.SIGWINCH, saved_handler) + + +@pytest.mark.skipif(os.name == 'nt', reason='SIGWINCH is POSIX-only') +def test_sigwinch_dispatch_survives_a_raising_bar( + isolated_resize_registry, caplog +) -> None: + # A35: one bar's failing `_handle_resize` skipped every later bar + # and raised out of the signal handler into whatever the main + # thread was doing. + class Broken(progressbar.ProgressBar): + def _handle_resize(self, signum=None, frame=None) -> None: + if signum is not None: + raise RuntimeError('spam') + super()._handle_resize(signum, frame) + + broken = Broken(max_value=5, fd=io.StringIO()) + broken.start() + other = progressbar.ProgressBar(max_value=5, fd=io.StringIO()) + other.start() + other.term_width = 1 + try: + with caplog.at_level(logging.WARNING, logger='progressbar.bar'): + signal.raise_signal(signal.SIGWINCH) + assert other.term_width != 1 + assert any( + record.levelno == logging.WARNING and record.exc_info + for record in caplog.records + ) + finally: + broken.finish() + other.finish() + + +@pytest.mark.skipif(os.name == 'nt', reason='SIGWINCH is POSIX-only') +def test_sigwinch_restored_after_a_leaked_bar( + isolated_resize_registry, +) -> None: + # A35: a bar that was never finished dropped out of the WeakSet, + # and nobody restored the handler it had installed. Registration + # happens at construction, so a bar that is built and dropped + # without start() is the shape that gets collected (a started bar + # is kept alive by `streams.listeners` until it finishes). + bar = progressbar.ProgressBar(max_value=5, fd=io.StringIO()) + assert signal.getsignal(signal.SIGWINCH) is not signal.SIG_IGN + bar._finished = True # keep __del__ from finishing it for us + del bar + gc.collect() + assert signal.getsignal(signal.SIGWINCH) is signal.SIG_IGN + + +@pytest.mark.skipif(os.name == 'nt', reason='SIGWINCH is POSIX-only') +def test_sigwinch_previous_handler_survives_an_off_thread_finish( + isolated_resize_registry, +) -> None: + # A35: `signal.signal` raises ValueError off the main thread, the + # bar was already unregistered, and the next bar then saved the + # registry's own handler as the "previous" one, losing SIG_IGN. + import threading + + first = progressbar.ProgressBar(max_value=5, fd=io.StringIO()) + first.start() + thread = threading.Thread(target=first.finish) + thread.start() + thread.join(10) + assert not thread.is_alive() + + second = progressbar.ProgressBar(max_value=5, fd=io.StringIO()) + second.start() + second.finish() + assert signal.getsignal(signal.SIGWINCH) is signal.SIG_IGN + + +@pytest.mark.skipif(os.name == 'nt', reason='SIGWINCH is POSIX-only') +def test_sigwinch_restores_the_default_when_no_python_handler_was_set( + isolated_resize_registry, +) -> None: + # A35: `signal.getsignal` returns None for a handler installed from + # C, and `signal.signal(SIGWINCH, None)` is a TypeError, so the + # registry's handler stayed installed for good. + registry = isolated_resize_registry + bar = progressbar.ProgressBar(max_value=5, fd=io.StringIO()) + bar.start() + registry.previous_handler = None + bar.finish() + assert signal.getsignal(signal.SIGWINCH) is signal.SIG_DFL diff --git a/tests/test_progressbar_command.py b/tests/test_progressbar_command.py index 65807abe..e2dc6677 100644 --- a/tests/test_progressbar_command.py +++ b/tests/test_progressbar_command.py @@ -1,3 +1,4 @@ +import argparse import io import pathlib @@ -278,7 +279,7 @@ def write(self, data) -> int: monkeypatch.setattr( main, '_get_output_stream', lambda *args: BrokenPipeIO() ) - main.main([str(file)]) # must not raise + main.main([str(file)]) # must not raise, and must not exit non-zero def test_main_empty_file_has_known_size(tmp_path, recorded_bars) -> None: @@ -301,3 +302,403 @@ def test_console_scripts_include_the_bar_shorthand() -> None: assert "progressbar = 'progressbar.__main__:main'" in pyproject assert "bar = 'progressbar.__main__:main'" in pyproject + + +def test_missing_input_keeps_existing_output(tmp_path: pathlib.Path) -> None: + output: pathlib.Path = tmp_path / 'out.bin' + output.write_bytes(b'precious') + with pytest.raises(SystemExit) as excinfo: + main.main([str(tmp_path / 'missing'), '-o', str(output)]) + assert excinfo.value.code == 2 + assert output.read_bytes() == b'precious' + + +def test_directory_input_is_rejected(tmp_path: pathlib.Path) -> None: + output: pathlib.Path = tmp_path / 'out.bin' + output.write_bytes(b'precious') + with pytest.raises(SystemExit) as excinfo: + main.main([str(tmp_path), '-o', str(output)]) + assert excinfo.value.code == 2 + assert output.read_bytes() == b'precious' + + +def test_output_same_as_input_is_rejected(tmp_path: pathlib.Path) -> None: + file: pathlib.Path = tmp_path / 'data.bin' + file.write_bytes(b'precious') + with pytest.raises(SystemExit) as excinfo: + main.main([str(file), '-o', str(file)]) + assert excinfo.value.code == 2 + assert file.read_bytes() == b'precious' + + +@pytest.mark.parametrize( + 'option', + [ + ['-B', '0'], + ['-B', 'abc'], + ['-s', 'x'], + ['-L', 'abc'], + ['-B', '99999999999999999999999'], # OverflowError in read() before + ], +) +def test_invalid_size_options_are_rejected( + tmp_path: pathlib.Path, option: list[str] +) -> None: + file: pathlib.Path = tmp_path / 'data.bin' + file.write_bytes(b'x' * 16) + output: pathlib.Path = tmp_path / 'out.bin' + output.write_bytes(b'precious') + with pytest.raises(SystemExit) as excinfo: + main.main([*option, str(file), '-o', str(output)]) + assert excinfo.value.code == 2 + assert output.read_bytes() == b'precious' + + +def test_keyboard_interrupt_exits_130( + tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch +) -> None: + file: pathlib.Path = tmp_path / 'data.bin' + file.write_bytes(b'x' * 4096) + + class InterruptingIO(io.BytesIO): + def write(self, data: object) -> int: + raise KeyboardInterrupt + + monkeypatch.setattr( + main, '_get_output_stream', lambda *args: InterruptingIO() + ) + with pytest.raises(SystemExit) as excinfo: + main.main([str(file)]) + assert excinfo.value.code == main.EXIT_INTERRUPTED + + +def test_inputs_are_closed_after_their_copy( + tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch +) -> None: + files: list[pathlib.Path] = [] + for index in range(3): + file: pathlib.Path = tmp_path / f'data{index}.bin' + file.write_bytes(b'x' * 8) + files.append(file) + + opened: list[io.BufferedReader] = [] + open_counts: list[int] = [] + original_open = pathlib.Path.open + + def recording_open(self: pathlib.Path, *args: object, **kwargs: object): + open_counts.append(sum(not stream.closed for stream in opened)) + stream = original_open(self, *args, **kwargs) # type: ignore[arg-type] + opened.append(stream) + return stream + + monkeypatch.setattr(pathlib.Path, 'open', recording_open) + main.main([*map(str, files), '-o', str(tmp_path / 'out.bin')]) + assert open_counts == [0, 0, 0] + assert all(stream.closed for stream in opened) + assert (tmp_path / 'out.bin').read_bytes() == b'x' * 24 + + +def test_output_directory_is_rejected(tmp_path: pathlib.Path) -> None: + source: pathlib.Path = tmp_path / 'in.bin' + source.write_bytes(b'data') + with pytest.raises(SystemExit) as excinfo: + main.main([str(source), '-o', str(tmp_path)]) + assert excinfo.value.code == 2 + + +def test_existing_output_distinct_from_inputs_is_overwritten( + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + source: pathlib.Path = tmp_path / 'in.bin' + source.write_bytes(b'new') + output: pathlib.Path = tmp_path / 'out.bin' + output.write_bytes(b'old contents') + monkeypatch.setattr('sys.stdin', io.TextIOWrapper(io.BytesIO(b''))) + main.main([str(source), '-', '-o', str(output)]) + assert output.read_bytes() == b'new' + + +def test_empty_size_option_means_not_given( + tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch +) -> None: + source: pathlib.Path = tmp_path / 'in.bin' + source.write_bytes(b'data') + output: pathlib.Path = tmp_path / 'out.bin' + monkeypatch.setattr('sys.stdin', io.TextIOWrapper(io.BytesIO(b''))) + main.main( + ['-q', '-s', '', '-B', '', '-L', '', str(source), '-o', str(output)] + ) + assert output.read_bytes() == b'data' + + +def test_output_on_a_device_shared_with_an_input_is_allowed() -> None: + if not pathlib.Path('/dev/null').exists(): + pytest.skip('no /dev/null on this platform') + main.main(['-q', '/dev/null', '-o', '/dev/null']) # must not exit 2 + + +@pytest.mark.parametrize('mode', [[], ['-l']]) +def test_pipeline_reader_closing_early_exits_zero( + tmp_path: pathlib.Path, mode: list[str] +) -> None: + import os + import subprocess + import sys + + if not hasattr(os, 'fork'): + pytest.skip('needs a POSIX pipe') + source: pathlib.Path = tmp_path / 'big.bin' + source.write_bytes(b'line of text\n' * 200_000) + env: dict[str, str] = dict(os.environ) + env['PYTHONPATH'] = str(pathlib.Path(progressbar.__file__).parents[1]) + proc = subprocess.Popen( + [sys.executable, '-m', 'progressbar', '-q', *mode, str(source)], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + env=env, + ) + assert proc.stdout is not None + proc.stdout.read(1) + proc.stdout.close() # the reader goes away, as `| head -c 1` does + _, stderr = proc.communicate(timeout=60) + assert proc.returncode == 0, stderr.decode() + assert b'Exception ignored' not in stderr + + +def test_broken_pipe_on_stdout_discards_the_descriptor( + tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch +) -> None: + import io + + file: pathlib.Path = tmp_path / 'data.bin' + file.write_bytes(b'x' * 1024) + + class BrokenStdout(io.BytesIO): + def write(self, data) -> int: + raise BrokenPipeError + + broken: BrokenStdout = BrokenStdout() + discarded: list[bool] = [] + monkeypatch.setattr(main, '_get_output_stream', lambda *args: broken) + monkeypatch.setattr(main.sys, 'stdout', broken) + monkeypatch.setattr( + main, '_discard_broken_stdout', lambda *args: discarded.append(True) + ) + main.main([str(file)]) + assert discarded == [True] + + +def test_discard_broken_stdout_points_fd_at_devnull( + monkeypatch: pytest.MonkeyPatch, +) -> None: + import os + + read_end, write_end = os.pipe() + os.close(read_end) # the reader is gone + saved_stdout: int = os.dup(1) + os.dup2(write_end, 1) + + class FakeStdout: + def fileno(self) -> int: + return 1 + + monkeypatch.setattr(main.sys, 'stdout', FakeStdout()) + try: + main._discard_broken_stdout(main.sys.stdout) + os.write(1, b'late output') # would raise EPIPE before + assert os.fstat(1).st_ino == os.stat(os.devnull).st_ino + finally: + os.dup2(saved_stdout, 1) + os.close(saved_stdout) + os.close(write_end) + + +def test_discard_broken_stdout_leaves_other_descriptors_alone( + monkeypatch: pytest.MonkeyPatch, +) -> None: + import io + import os + + read_end, write_end = os.pipe() + + class PipeStdout: # an application's own pager pipe installed as stdout + def fileno(self) -> int: + return write_end + + monkeypatch.setattr(main.sys, 'stdout', PipeStdout()) + try: + main._discard_broken_stdout(main.sys.stdout) + os.write(write_end, b'XYZ') + assert os.read(read_end, 3) == b'XYZ' # still the pipe, not devnull + finally: + os.close(read_end) + os.close(write_end) + + monkeypatch.setattr(main.sys, 'stdout', io.StringIO()) + main._discard_broken_stdout(main.sys.stdout) # no fileno(): no raise + + +def test_output_that_is_also_stdin_is_rejected( + tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch +) -> None: + import io + + file: pathlib.Path = tmp_path / 'data.bin' + file.write_bytes(b'preserve this') + with file.open('rb') as stdin_file: + with pytest.raises(SystemExit) as excinfo: + main._check_output_path( + argparse.ArgumentParser(), str(file), [stdin_file] + ) + assert excinfo.value.code == 2 + assert file.read_bytes() == b'preserve this' + # a stream without a descriptor is not an alias of anything + main._check_output_path( + argparse.ArgumentParser(), str(file), [io.BytesIO()] + ) + + +def test_broken_pipe_found_at_flush_time_is_handled( + tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch +) -> None: + import io + + file: pathlib.Path = tmp_path / 'small.bin' + file.write_bytes(b'12345') + + class LateBrokenPipe(io.BytesIO): + def flush(self) -> None: + raise BrokenPipeError + + stream: LateBrokenPipe = LateBrokenPipe() + discarded: list[bool] = [] + monkeypatch.setattr(main, '_get_output_stream', lambda *args: stream) + monkeypatch.setattr(main.sys, 'stdout', stream) + monkeypatch.setattr( + main, '_discard_broken_stdout', lambda *args: discarded.append(True) + ) + main.main([str(file)]) # must not raise, and must not exit non-zero + assert discarded == [True] + + +def test_closed_stdin_is_a_usage_error( + tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch +) -> None: + output: pathlib.Path = tmp_path / 'out.bin' + output.write_bytes(b'precious') + monkeypatch.setattr(main.sys, 'stdin', None) # `exec 0<&-` shape + with pytest.raises(SystemExit) as excinfo: + main.main(['-', '-o', str(output)]) + assert excinfo.value.code == 2 + assert output.read_bytes() == b'precious' + + +@pytest.mark.parametrize( + ('line_mode', 'closed_part'), + [(False, 'text'), (True, 'text'), (False, 'binary')], +) +def test_closed_stdin_stream_leaves_output_untouched( + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, + line_mode: bool, + closed_part: str, +) -> None: + output: pathlib.Path = tmp_path / 'out.bin' + output.write_bytes(b'precious') + binary: io.BytesIO = io.BytesIO(b'input') + stdin: io.StringIO | io.TextIOWrapper + if closed_part == 'text': + stdin = io.TextIOWrapper(binary) + stdin.close() + else: + # A replacement stdin can expose a closed binary stream while + # its text wrapper remains open. + class SeparateStdin(io.StringIO): + buffer: io.BytesIO = binary + + stdin = SeparateStdin('input') + binary.close() + monkeypatch.setattr(main.sys, 'stdin', stdin) + with pytest.raises(SystemExit) as excinfo: + main.main([*(['-l'] if line_mode else []), '-', '-o', str(output)]) + assert excinfo.value.code == 2 + assert output.read_bytes() == b'precious' + + +def test_unreadable_input_leaves_the_output_untouched( + tmp_path: pathlib.Path, +) -> None: + import os + + if os.name == 'nt' or os.geteuid() == 0: + pytest.skip('needs POSIX permissions and a non-root user') + secret: pathlib.Path = tmp_path / 'secret.txt' + secret.write_bytes(b'hidden') + secret.chmod(0) + output: pathlib.Path = tmp_path / 'out.txt' + output.write_bytes(b'precious') + try: + with pytest.raises(SystemExit) as excinfo: + main.main(['-q', str(secret), '-o', str(output)]) + finally: + secret.chmod(0o600) + assert excinfo.value.code == 2 + assert output.read_bytes() == b'precious' + + +@pytest.mark.skipif( + not pathlib.Path('/dev/stdout').exists(), reason='needs /dev/stdout' +) +def test_output_path_aliasing_stdout_exits_zero_on_broken_pipe( + tmp_path: pathlib.Path, +) -> None: + # `-o /dev/stdout | head -c1` ended with status 1: the file opened + # for `-o` is not `sys.stdout`, so the broken-pipe handling for the + # reader going away did not apply to it. + import subprocess + import sys + + file = tmp_path / 'data.bin' + file.write_bytes(b'x' * (4 * 1024 * 1024)) + process = subprocess.Popen( + [sys.executable, '-m', 'progressbar', str(file), '-o', '/dev/stdout'], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + env={ + **__import__('os').environ, + 'PYTHONPATH': str(pathlib.Path(__file__).parents[1]), + }, + ) + assert process.stdout is not None + assert process.stdout.read(1) == b'x' + process.stdout.close() + _, stderr = process.communicate(timeout=60) + assert process.returncode == 0, stderr.decode(errors='replace') + assert b'Exception ignored' not in stderr + + +def test_output_is_stdout_without_a_descriptor_backed_stdout( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # A stdout without a descriptor (a StringIO under a test runner) can + # alias nothing, so an -o file is never mistaken for it + monkeypatch.setattr(main.sys, 'stdout', io.StringIO()) + assert main._output_is_stdout(io.BytesIO()) is False + + +@pytest.mark.skipif( + not pathlib.Path('/dev/fd/1').exists(), reason='needs /dev/fd' +) +def test_output_is_stdout_for_a_path_that_aliases_descriptor_one( + monkeypatch: pytest.MonkeyPatch, tmp_path: pathlib.Path +) -> None: + class DescriptorOneStdout: + def fileno(self) -> int: + return 1 + + monkeypatch.setattr(main.sys, 'stdout', DescriptorOneStdout()) + with open('/dev/fd/1', 'wb') as alias: # noqa: PTH123 + assert main._output_is_stdout(alias) is True + with open(tmp_path / 'out.bin', 'wb') as regular: # noqa: PTH123 + assert main._output_is_stdout(regular) is False diff --git a/tests/test_stream_security.py b/tests/test_stream_security.py new file mode 100644 index 00000000..e5be65d3 --- /dev/null +++ b/tests/test_stream_security.py @@ -0,0 +1,1390 @@ +from __future__ import annotations + +import inspect +import io +import logging +import subprocess +import sys +import textwrap +import threading +import typing +from collections.abc import Iterator + +import pytest +from test_stream import reset_wrapped_streams + +import progressbar +from progressbar import utils + + +@pytest.fixture(autouse=True) +def clean_streams() -> Iterator[None]: + reset_wrapped_streams() + yield + reset_wrapped_streams() + + +class _SelfRemovingListener: + def __init__(self, listeners: set[object]) -> None: + self.listeners: set[object] = listeners + self.calls: int = 0 + + def update(self) -> None: + self.calls += 1 + self.listeners.discard(self) + self.listeners.add(_NoopListener()) + + +class _NoopListener: + def update(self) -> None: + pass + + +def test_listener_mutation_during_write_does_not_raise() -> None: + target: io.StringIO = io.StringIO() + listeners: set[object] = set() + listener: _SelfRemovingListener = _SelfRemovingListener(listeners) + listeners.add(listener) + wrapper: utils.WrappingIO = utils.WrappingIO( + target, + capturing=True, + listeners=listeners, # type: ignore[arg-type] + ) + + wrapper.write('line\n') + + assert listener.calls == 1 + assert listener not in listeners + + +def test_empty_listener_set_is_shared() -> None: + listeners: set[object] = set() + wrapper: utils.WrappingIO = utils.WrappingIO( + io.StringIO(), + listeners=listeners, # type: ignore[arg-type] + ) + assert wrapper.listeners is listeners + + +def test_stop_capturing_never_goes_negative() -> None: + streams: utils.StreamWrapper = progressbar.streams + assert streams.capturing == 0 + streams.stop_capturing() + streams.stop_capturing() + assert streams.capturing == 0 + streams.start_capturing() + assert streams.capturing == 1 + streams.stop_capturing() + assert streams.capturing == 0 + + +def test_unwrap_while_capturing_flushes_dropped_wrapper( + monkeypatch: pytest.MonkeyPatch, +) -> None: + streams: utils.StreamWrapper = progressbar.streams + real: io.StringIO = io.StringIO() + monkeypatch.setattr(streams, 'original_stdout', real) + + wrapper: utils.WrappingIO = streams.wrap_stdout() + streams.start_capturing() + try: + assert wrapper.capturing + sys.stdout.write('held\n') + assert real.getvalue() == '' + + streams.unwrap_stdout() + + assert sys.stdout is real + assert not wrapper.capturing + assert wrapper.buffer.getvalue() == '' + assert real.getvalue() == 'held\n' + assert not streams.wrapped_excepthook + finally: + streams.stop_capturing() + + +def test_excepthook_flag_is_boolean() -> None: + streams: utils.StreamWrapper = progressbar.streams + streams.wrap_stdout() + streams.wrap_stderr() + assert streams.wrapped_excepthook is True + streams.unwrap_stdout() + assert streams.wrapped_excepthook is True + streams.unwrap_stderr() + assert streams.wrapped_excepthook is False + + +def test_unwrap_stdout_without_wrapper_restores_original() -> None: + streams: utils.StreamWrapper = progressbar.streams + assert not isinstance(streams.stdout, utils.WrappingIO) + streams.unwrap_stdout() + assert streams.wrapped_stdout == 0 + assert sys.stdout is streams.original_stdout + + +# ---- round-3 take-backs ------------------------------------------------- + + +def test_wrapper_snapshots_listeners_under_the_shared_lock() -> None: + streams: utils.StreamWrapper = progressbar.streams + wrapper: utils.WrappingIO = streams.wrap_stdout() + try: + assert wrapper.lock is streams.listeners_lock + finally: + streams.unwrap_stdout() + + +def test_standalone_wrapper_has_its_own_lock() -> None: + wrapper: utils.WrappingIO = utils.WrappingIO(io.StringIO()) + assert wrapper.lock.acquire(blocking=False) + wrapper.lock.release() + + +def test_streamwrapper_survives_deepcopy_and_pickle( + monkeypatch: pytest.MonkeyPatch, +) -> None: + import copy + import pickle + + monkeypatch.setattr(sys, 'stdout', io.StringIO()) + monkeypatch.setattr(sys, 'stderr', io.StringIO()) + original: utils.StreamWrapper = utils.StreamWrapper() + clone: utils.StreamWrapper = copy.deepcopy(original) + assert clone.lock is not original.lock + assert clone.lock.acquire(blocking=False) + clone.lock.release() + restored: utils.StreamWrapper = pickle.loads(pickle.dumps(original)) + assert restored.lock.acquire(blocking=False) + restored.lock.release() + + +def test_unwrap_tolerates_a_stream_without_flush( + monkeypatch: pytest.MonkeyPatch, +) -> None: + class WriteOnly: + def write(self, text: str) -> int: + return len(text) + + streams: utils.StreamWrapper = progressbar.streams + target: WriteOnly = WriteOnly() + monkeypatch.setattr(streams, 'original_stdout', target) + monkeypatch.setattr(streams, 'stdout', target) + monkeypatch.setattr(sys, 'stdout', target) + streams.wrap_stdout() + streams.unwrap_stdout() # must not raise + assert sys.stdout is target + + +def test_subclass_without_super_init_can_wrap( + monkeypatch: pytest.MonkeyPatch, +) -> None: + class NoSuper(utils.StreamWrapper): + def __init__(self, out: io.StringIO) -> None: + self.stdout = self.original_stdout = out + self.stderr = self.original_stderr = io.StringIO() + self.original_excepthook = sys.excepthook + self.listeners = set() + self.logging_handlers = [] + + hook = sys.excepthook + wrapper: NoSuper = NoSuper(io.StringIO()) + try: + wrapper.wrap_stdout() + finally: + wrapper.unwrap_stdout() + monkeypatch.setattr(sys, 'excepthook', hook) + + +# ---- round-3 closing fixes --------------------------------------------- + + +def test_wrapping_io_and_a_redirecting_bar_deepcopy_and_pickle( + monkeypatch: pytest.MonkeyPatch, +) -> None: + import copy + import pickle + + wrapper: utils.WrappingIO = utils.WrappingIO(io.StringIO()) + clone: utils.WrappingIO = copy.deepcopy(wrapper) + assert clone.lock is not wrapper.lock + assert clone.lock.acquire(blocking=False) + clone.lock.release() + restored: utils.WrappingIO = pickle.loads(pickle.dumps(wrapper)) + assert restored.lock.acquire(blocking=False) + restored.lock.release() + + streams: utils.StreamWrapper = progressbar.streams + target: io.StringIO = io.StringIO() + errors: io.StringIO = io.StringIO() + monkeypatch.setattr(streams, 'original_stdout', target) + monkeypatch.setattr(streams, 'stdout', target) + monkeypatch.setattr(sys, 'stdout', target) + monkeypatch.setattr(streams, 'original_stderr', errors) + monkeypatch.setattr(streams, 'stderr', errors) + monkeypatch.setattr(sys, 'stderr', errors) + bar: progressbar.ProgressBar = progressbar.ProgressBar( + max_value=5, redirect_stdout=True, fd=io.StringIO() + ) + bar.start() + try: + clones: list[object] = [copy.deepcopy(bar), copy.deepcopy(streams)] + finally: + bar.finish() + # A copy of a started bar is a started bar: its __del__ would finish() + # it later, at gc time, and unwrap the singleton under some other test + for clone in clones: + for listener in getattr(clone, 'listeners', [clone]): + listener._finished = True + + +def test_captured_write_never_waits_for_the_streamwrapper_lock() -> None: + import threading + + streams: utils.StreamWrapper = progressbar.streams + wrapper: utils.WrappingIO = streams.wrap_stdout() + try: + assert wrapper.lock is streams.listeners_lock + assert wrapper.lock is not streams.lock + streams.lock.acquire() + done: threading.Event = threading.Event() + + def write() -> None: + wrapper.capturing = True + wrapper.write('line\n') + done.set() + + thread: threading.Thread = threading.Thread(target=write) + thread.start() + assert done.wait(2.0), 'write() blocked on streams.lock' + thread.join() + finally: + streams.lock.release() + streams.unwrap_stdout() + + +def test_unwrap_tolerates_a_stream_whose_flush_raises_anything( + monkeypatch: pytest.MonkeyPatch, +) -> None: + class DeletedWidget: # PyQt 'wrapped C/C++ object has been deleted' shape + closed: bool = False + + def write(self, text: str) -> int: + return len(text) + + def flush(self) -> None: + raise RuntimeError('wrapped C/C++ object has been deleted') + + streams: utils.StreamWrapper = progressbar.streams + target: DeletedWidget = DeletedWidget() + monkeypatch.setattr(streams, 'original_stdout', target) + monkeypatch.setattr(streams, 'stdout', target) + monkeypatch.setattr(sys, 'stdout', target) + streams.wrap_stdout() + streams.unwrap_stdout() # must not raise + assert sys.stdout is target + + +# ---- second review round (Codex findings 1-4) --------------------------- + + +class _LockProbeHandler(logging.Handler): + """Record, per log record, whether `lock` was free at emit time.""" + + def __init__(self, lock: threading.RLock) -> None: + super().__init__(level=logging.DEBUG) + self.watched = lock # not `self.lock`: that is the handler's own + self.free: list[bool] = [] + + def emit(self, record: logging.LogRecord) -> None: + result: list[bool] = [] + + def probe() -> None: + got: bool = self.watched.acquire(timeout=1.0) + if got: + self.watched.release() + result.append(got) + + thread: threading.Thread = threading.Thread(target=probe) + thread.start() + thread.join() + self.free.append(result[0]) + + +def test_logging_never_happens_under_the_transition_lock( + monkeypatch: pytest.MonkeyPatch, +) -> None: + class RaisingFlush(io.StringIO): + armed: bool = False + + def flush(self) -> None: + if self.armed: + raise RuntimeError('gone') + + streams: utils.StreamWrapper = utils.StreamWrapper() + original: RaisingFlush = RaisingFlush() + streams.original_stdout = original + streams.stdout = original + monkeypatch.setattr(sys, 'stdout', original) + handler: _LockProbeHandler = _LockProbeHandler(streams.lock) + logger: logging.Logger = logging.getLogger('progressbar.utils') + logger.addHandler(handler) + old_level: int = logger.level + logger.setLevel(logging.DEBUG) + try: + streams.wrap_stdout() # logs 'wrapping excepthook' + streams.start_capturing() + print('buffered') + original.armed = True + streams.unwrap_stdout() # logs 'Could not deliver ...' + streams.stop_capturing() + finally: + logger.removeHandler(handler) + logger.setLevel(old_level) + assert len(handler.free) >= 2 + assert all(handler.free), handler.free + + +def test_targets_are_never_written_under_the_transition_lock( + monkeypatch: pytest.MonkeyPatch, +) -> None: + streams: utils.StreamWrapper = utils.StreamWrapper() + seen: list[bool] = [] + + class LockProbeTarget(io.StringIO): + def write(self, text: str) -> int: + result: list[bool] = [] + + def probe() -> None: + got: bool = streams.lock.acquire(timeout=1.0) + if got: + streams.lock.release() + result.append(got) + + thread: threading.Thread = threading.Thread(target=probe) + thread.start() + thread.join() + seen.append(result[0]) + return super().write(text) + + target: LockProbeTarget = LockProbeTarget() + streams.original_stdout = target + streams.stdout = target + monkeypatch.setattr(sys, 'stdout', target) + streams.wrap_stdout() + streams.start_capturing() + print('one') + streams.stop_capturing() # delivers 'one' to the target + streams.start_capturing() + print('two') + streams.unwrap_stdout() # delivers 'two' to the target + assert target.getvalue() == 'one\ntwo\n' + assert len(seen) >= 2 + assert all(seen), seen + + +class NamedIO(utils.WrappingIO): # pickle needs a module-level class + __slots__ = ('name',) + + def __init__(self, target: io.StringIO, name: str) -> None: + super().__init__(target) + self.name = name + + +class NamedStreams(utils.StreamWrapper): + __slots__ = ('name',) + + def __init__(self, name: str) -> None: + super().__init__() + self.name = name + + +def test_pickle_and_deepcopy_keep_subclass_slots() -> None: + import copy + import pickle + + named_io: NamedIO = NamedIO(io.StringIO(), 'spam-stream') + named_streams: NamedStreams = NamedStreams('spam-streams') + for name in ('stdout', 'stderr'): + setattr(named_streams, f'original_{name}', io.StringIO()) + setattr(named_streams, name, io.StringIO()) + # a builtin pickles by reference only while it is still installed, + # and another test may have left the shared hook wrapped + named_streams.original_excepthook = sys.excepthook + for restore in (copy.deepcopy, lambda o: pickle.loads(pickle.dumps(o))): + assert restore(named_io).name == 'spam-stream' + assert restore(named_streams).name == 'spam-streams' + + +def test_restored_wrappers_share_the_restored_listener_lock( + monkeypatch: pytest.MonkeyPatch, +) -> None: + import copy + import pickle + + streams: utils.StreamWrapper = utils.StreamWrapper() + for name in ('stdout', 'stderr'): + target: io.StringIO = io.StringIO() + setattr(streams, f'original_{name}', target) + setattr(streams, name, target) + monkeypatch.setattr(sys, name, target) + streams.wrap_stdout() + streams.wrap_stderr() + streams.original_excepthook = sys.excepthook # see the slots test + try: + for restored in ( + copy.deepcopy(streams), + pickle.loads(pickle.dumps(streams)), + ): + assert isinstance(restored.stdout, utils.WrappingIO) + assert isinstance(restored.stderr, utils.WrappingIO) + assert restored.stdout.listeners is restored.listeners + assert restored.stdout.lock is restored.listeners_lock + assert restored.stderr.lock is restored.listeners_lock + assert restored.listeners_lock is not streams.listeners_lock + finally: + streams.unwrap_stdout() + streams.unwrap_stderr() + + +def test_wrapping_io_subclass_without_super_init_can_write() -> None: + class NoSuper(utils.WrappingIO): + def __init__(self, target: io.StringIO) -> None: + self.buffer = io.StringIO() + self.target = target + self.capturing = True + self.listeners = set() + self.needs_clear = False + + wrapper: NoSuper = NoSuper(io.StringIO()) + assert wrapper.write('spam\n') == 5 + assert wrapper.needs_clear + + +def test_stream_class_type_hints_evaluate_on_this_python() -> None: + import typing + + # `threading.Lock` is a factory function before 3.13, so an annotation + # naming it directly cannot be evaluated there (regression caught by + # the second hunt). + typing.get_type_hints(utils.WrappingIO.__init__) + typing.get_type_hints(utils.WrappingIO) + typing.get_type_hints(utils.StreamWrapper.__init__) + + +def test_forked_child_gets_fresh_stream_locks() -> None: + import os + + if not hasattr(os, 'fork') or not hasattr(os, 'register_at_fork'): + pytest.skip('needs a POSIX fork') + streams: utils.StreamWrapper = progressbar.streams + held: threading.Event = threading.Event() + release: threading.Event = threading.Event() + + def holder() -> None: + with streams.lock, streams.listeners_lock: + held.set() + release.wait(10) + + thread: threading.Thread = threading.Thread(target=holder, daemon=True) + thread.start() + held.wait(5) + pid: int = os.fork() + if pid == 0: # pragma: no cover - child + status: int = 0 + if not streams.lock.acquire(timeout=2): + status |= 1 + if not streams.listeners_lock.acquire(timeout=2): + status |= 2 + os._exit(status) + try: + _, raw_status = os.waitpid(pid, 0) + finally: + release.set() + thread.join() + assert os.waitstatus_to_exitcode(raw_status) == 0 + + +def test_wrap_excepthook_directly(monkeypatch: pytest.MonkeyPatch) -> None: + streams: utils.StreamWrapper = utils.StreamWrapper() + original: object = sys.excepthook + monkeypatch.setattr(sys, 'excepthook', original) + streams.original_excepthook = original # type: ignore[assignment] + streams.wrap_excepthook() + assert sys.excepthook == streams.excepthook + streams.wrap_excepthook() # idempotent + streams.unwrap_excepthook() + assert sys.excepthook is original + + +# ---- third review round (Codex re-review findings 1-4) ------------------- + + +def test_overlapping_stop_capturing_delivers_buffered_text_once( + monkeypatch: pytest.MonkeyPatch, +) -> None: + paused: threading.Event = threading.Event() + resume: threading.Event = threading.Event() + + streams: utils.StreamWrapper = utils.StreamWrapper() + target: io.StringIO = io.StringIO() + streams.stdout = streams.original_stdout = target + monkeypatch.setattr(sys, 'stdout', target) + wrapper: utils.WrappingIO = streams.wrap_stdout() + streams.start_capturing() + wrapper.write('spam') + + def stop() -> None: + sys.settrace(_pause_native_drain(paused, resume)) + try: + streams.stop_capturing() + finally: + sys.settrace(None) + + first: threading.Thread = threading.Thread(target=stop, name='first-stop') + first.start() + assert paused.wait(10) + second_done: threading.Event = threading.Event() + + def another_lifecycle() -> None: + streams.start_capturing() + streams.stop_capturing() + second_done.set() + + second: threading.Thread = threading.Thread(target=another_lifecycle) + second.start() + second_done.wait(0.3) + resume.set() + first.join() + second.join() + streams.unwrap_stdout() + assert target.getvalue() == 'spam' + + +def test_subclass_defining_eq_without_hash_constructs() -> None: + class EqualStreams(utils.StreamWrapper): + def __eq__(self, other: object) -> bool: + return self is other + + streams: EqualStreams = EqualStreams() + assert streams.lock is not utils.StreamWrapper.lock + + +class _BackReference: # a listener that refers to its StreamWrapper + def __init__(self, streams: utils.StreamWrapper) -> None: + self.streams = streams + + def update(self) -> None: + pass + + +def test_cyclic_graph_restores_from_any_root( + monkeypatch: pytest.MonkeyPatch, +) -> None: + import copy + import pickle + + streams: utils.StreamWrapper = utils.StreamWrapper() + for name in ('stdout', 'stderr'): + target: io.StringIO = io.StringIO() + setattr(streams, f'original_{name}', target) + setattr(streams, name, target) + monkeypatch.setattr(sys, name, target) + streams.original_excepthook = sys.excepthook + listener: _BackReference = _BackReference(streams) + wrapper: utils.WrappingIO = streams.wrap_stdout() + streams.start_capturing(listener) # type: ignore[arg-type] + try: + for root in (streams, wrapper, listener): + for restore in ( + copy.deepcopy, + lambda o: pickle.loads(pickle.dumps(o)), + ): + clone: object = restore(root) + if isinstance(clone, utils.WrappingIO): + owner = next(iter(clone.listeners)).streams # type: ignore[attr-defined] + assert clone.lock is owner.listeners_lock + elif isinstance(clone, _BackReference): + owner = clone.streams + else: + owner = clone + assert isinstance(owner.stdout, utils.WrappingIO) + assert owner.stdout.lock is owner.listeners_lock + assert owner.listeners_lock is not streams.listeners_lock + finally: + streams.stop_capturing(listener) # type: ignore[arg-type] + streams.unwrap_stdout() + + +def _fork_and_acquire(current_lock: typing.Callable[[], typing.Any]) -> None: + # `current_lock` is re-read in the child: the fork hook replaces the + # lock object, so a reference taken before the fork stays locked + import os + + if not hasattr(os, 'fork'): + pytest.skip('needs a POSIX fork') + held: threading.Event = threading.Event() + release: threading.Event = threading.Event() + + def holder() -> None: + with current_lock(): + held.set() + release.wait(10) + + thread: threading.Thread = threading.Thread(target=holder, daemon=True) + thread.start() + held.wait(5) + pid: int = os.fork() + if pid == 0: # pragma: no cover - child + os._exit(0 if current_lock().acquire(timeout=2) else 1) + try: + _, raw_status = os.waitpid(pid, 0) + finally: + release.set() + thread.join() + assert os.waitstatus_to_exitcode(raw_status) == 0 + + +def test_forked_child_gets_a_fresh_standalone_wrapper_lock() -> None: + wrapper: utils.WrappingIO = utils.WrappingIO(io.StringIO()) + _fork_and_acquire(lambda: wrapper.lock) + + +def test_forked_child_gets_a_fresh_interval_warning_lock() -> None: + from progressbar import bar as bar_module + + _fork_and_acquire(lambda: bar_module._bad_interval_lock.lock) + + +def test_shared_lock_box_reset_and_wrapper_lock_setter() -> None: + box: utils._SharedLock = utils._SharedLock() + old_lock: object = box.lock + with box.lock: + box.reset() # what the fork hook does: a new, unlocked lock + assert box.lock is not old_lock + assert box.lock.acquire(blocking=False) + box.lock.release() + + wrapper: utils.WrappingIO = utils.WrappingIO(io.StringIO()) + private: threading.Lock = threading.Lock() + wrapper.lock = private # the 4.6.0-era way of handing in a lock + assert wrapper.lock is private + + +def test_stop_capturing_returns_with_its_own_text_delivered( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # Delivery is synchronous, as in 4.6.0: text written right after a + # stop lands below the text that stop flushed, even while another + # thread is still inside a delivery of its own + paused: threading.Event = threading.Event() + resume: threading.Event = threading.Event() + + class PausingTarget(io.StringIO): + def write(self, text: str) -> int: + written: int = super().write(text) + if threading.current_thread().name == 'first-stop': + paused.set() + assert resume.wait(10) + return written + + streams: utils.StreamWrapper = utils.StreamWrapper() + target: PausingTarget = PausingTarget() + streams.stdout = streams.original_stdout = target + monkeypatch.setattr(sys, 'stdout', target) + wrapper: utils.WrappingIO = streams.wrap_stdout() + streams.start_capturing() + wrapper.write('spam|') + + first: threading.Thread = threading.Thread( + target=streams.stop_capturing, name='first-stop' + ) + first.start() + assert paused.wait(10) + streams.start_capturing() + wrapper.write('eggs|') + streams.stop_capturing() # delivers 'eggs|' itself, without waiting + wrapper.write('after-stop|') # passes straight through, capturing is off + resume.set() + first.join() + streams.unwrap_stdout() + assert target.getvalue() == 'spam|eggs|after-stop|' + + +def test_target_that_flushes_the_streams_while_written_does_not_deadlock( + monkeypatch: pytest.MonkeyPatch, +) -> None: + streams: utils.StreamWrapper = utils.StreamWrapper() + + class FlushingTarget(io.StringIO): + # A target that calls back into the streams on the same thread, + # as a bar-aware widget or a logging handler might + def write(self, text: str) -> int: + written: int = super().write(text) + streams.flush() + return written + + target: FlushingTarget = FlushingTarget() + streams.stdout = streams.original_stdout = target + monkeypatch.setattr(sys, 'stdout', target) + streams.wrap_stdout() + streams.start_capturing() + print('spam', end='') + streams.stop_capturing() + streams.unwrap_stdout() + assert target.getvalue() == 'spam' + + +def test_failed_stdout_delivery_still_delivers_the_stderr_text( + monkeypatch: pytest.MonkeyPatch, +) -> None: + class Broken(io.StringIO): + def write(self, text: str) -> int: + raise OSError('gone') + + streams: utils.StreamWrapper = utils.StreamWrapper() + out: Broken = Broken() + err: io.StringIO = io.StringIO() + streams.stdout = streams.original_stdout = out + streams.stderr = streams.original_stderr = err + monkeypatch.setattr(sys, 'stdout', out) + monkeypatch.setattr(sys, 'stderr', err) + streams.wrap_stdout() + streams.wrap_stderr() + streams.start_capturing() + print('spam', end='') + print('eggs', end='', file=sys.stderr) + with pytest.raises(OSError, match='gone'): + streams.stop_capturing() + assert err.getvalue() == 'eggs' # not lost to the stdout failure + streams.unwrap_stdout() + streams.unwrap_stderr() + + +def _pause_native_drain( + paused: threading.Event, resume: threading.Event +) -> typing.Callable[..., typing.Any]: + """Pause real StringIO draining without replacing its methods.""" + lines, first = inspect.getsourcelines(utils.WrappingIO._take) + pause_line: int = first + next( + i for i, line in enumerate(lines) if 'self.buffer.truncate(0)' in line + ) + + def trace(frame: typing.Any, event: str, arg: typing.Any) -> typing.Any: + if ( + event == 'line' + and frame.f_code is utils.WrappingIO._take.__code__ + and frame.f_lineno == pause_line + and not paused.is_set() + ): + paused.set() + assert resume.wait(10) + return trace + + return trace + + +def test_write_during_a_drain_is_not_lost() -> None: + # Codex-4b finding 7: `_take()` read the buffer, then truncated it, + # and a `write()` that landed between the two was gone. + wrapper = utils.WrappingIO(io.StringIO(), capturing=True) + paused: threading.Event = threading.Event() + resume: threading.Event = threading.Event() + wrapper.write('A|') + taken: list[str] = [] + + def take() -> None: + sys.settrace(_pause_native_drain(paused, resume)) + try: + taken.append(wrapper._take()) + finally: + sys.settrace(None) + + taker = threading.Thread(target=take) + taker.start() + assert paused.wait(10) + writer = threading.Thread(target=wrapper.write, args=('B|',)) + writer.start() + writer.join(0.5) # blocked behind the drain, or done without a lock + resume.set() + taker.join(10) + writer.join(10) + assert not taker.is_alive() and not writer.is_alive() + + taken.append(wrapper._take()) + assert ''.join(taken) == 'A|B|' + + +def test_buffer_flush_that_logs_through_the_wrapper_does_not_deadlock() -> ( + None +): + # Review 5: `_take()` called `buffer.flush()` while holding the buffer + # lock. A replacement buffer whose flush() logs through a handler + # that writes to the wrapper then waited for the handler lock, held + # by a thread waiting to write into the buffer: a lock-order cycle + # 4.6.0 could not have. + entered: threading.Event = threading.Event() + handler_held: threading.Event = threading.Event() + logger: logging.Logger = logging.getLogger('test-buffer-flush-logs') + logger.propagate = False + logger.setLevel(logging.INFO) + + class LoggingBuffer(io.StringIO): + fired: bool = False + + def flush(self) -> None: + if not self.fired: + self.fired = True + entered.set() + logger.info('flush-log') + super().flush() + + wrapper = utils.WrappingIO(io.StringIO(), capturing=True) + wrapper.buffer = LoggingBuffer() + handler: logging.StreamHandler[typing.Any] = logging.StreamHandler(wrapper) + logger.addHandler(handler) + try: + wrapper.write('A|') + + def writer() -> None: + handler.acquire() + try: + handler_held.set() + assert entered.wait(10) + wrapper.write('B|') + finally: + handler.release() + + def drainer() -> None: + assert handler_held.wait(10) + wrapper._flush() + + threads = [ + threading.Thread(target=writer, daemon=True), + threading.Thread(target=drainer, daemon=True), + ] + for thread in threads: + thread.start() + for thread in threads: + thread.join(10) + assert not any(thread.is_alive() for thread in threads) + wrapper._flush() + assert wrapper.target.getvalue() == 'A|B|flush-log\n' + finally: + logger.removeHandler(handler) + + +@pytest.mark.parametrize( + 'action', ['flush', 'stop_capturing', 'unwrap_stdout'] +) +@pytest.mark.parametrize('transition', ['start_capturing', 'stop_capturing']) +def test_buffer_flush_logging_does_not_hold_transition_lock( + action: str, + transition: str, +) -> None: + # Isolate a real logging lock cycle so a regression cannot block + # pytest's logging shutdown, even when daemon threads are stuck. + script: str = textwrap.dedent("""\ + import io + import logging + import os + import sys + import threading + from progressbar import utils + + streams: utils.StreamWrapper = utils.StreamWrapper() + target: io.StringIO = io.StringIO() + streams.stdout = streams.original_stdout = target + wrapper: utils.WrappingIO = streams.wrap_stdout() + streams.start_capturing() + entered: threading.Event = threading.Event() + held: threading.Event = threading.Event() + logger: logging.Logger = logging.getLogger('buffer-transition') + logger.setLevel(logging.INFO) + logger.propagate = False + handler: logging.StreamHandler = logging.StreamHandler(io.StringIO()) + logger.addHandler(handler) + failures: list[BaseException] = [] + + class LoggingBuffer(io.StringIO): + fired: bool = False + + def flush(self) -> None: + if not self.fired: + self.fired = True + entered.set() + logger.info('buffer flush') + super().flush() + + wrapper.buffer = LoggingBuffer() + wrapper.write('pending') + + def writer() -> None: + handler.acquire() + try: + held.set() + assert entered.wait(3), 'buffer flush was not called' + getattr(streams, sys.argv[2])() + except BaseException as exception: + failures.append(exception) + finally: + handler.release() + + def drainer() -> None: + try: + assert held.wait(3) + getattr(streams, sys.argv[1])() + except BaseException as exception: + failures.append(exception) + + threads: list[threading.Thread] = [ + threading.Thread(target=writer, daemon=True), + threading.Thread(target=drainer, daemon=True), + ] + for thread in threads: + thread.start() + for thread in threads: + thread.join(4) + blocked: bool = any(thread.is_alive() for thread in threads) + delivered: bool = target.getvalue() == 'pending' + print(blocked, delivered, failures, file=sys.__stdout__, flush=True) + os._exit(1 if blocked or failures or not delivered else 0) + """) + result: subprocess.CompletedProcess[str] = subprocess.run( + [sys.executable, '-c', script, action, transition], + capture_output=True, + text=True, + timeout=12, + check=False, + ) + assert result.returncode == 0, result.stdout + result.stderr + + +def test_capture_transitions_notify_subclass_outside_locks( + monkeypatch: pytest.MonkeyPatch, +) -> None: + calls: list[tuple[int, bool]] = [] + unlocked: list[bool] = [] + + class CustomStreams(utils.StreamWrapper): + def update_capturing(self) -> None: + def check_lock() -> None: + acquired: bool = self.lock.acquire(timeout=1) + unlocked.append(acquired) + if acquired: + self.lock.release() + + observer: threading.Thread = threading.Thread(target=check_lock) + observer.start() + observer.join(2) + assert not observer.is_alive() + calls.append((self.capturing, self.stdout.capturing)) + super().update_capturing() + + streams: CustomStreams = CustomStreams() + target: io.StringIO = io.StringIO() + streams.stdout = streams.original_stdout = target + monkeypatch.setattr(sys, 'stdout', target) + wrapper: utils.WrappingIO = streams.wrap_stdout() + streams.start_capturing() + wrapper.write('pending') + streams.stop_capturing() + assert calls == [(1, True), (0, False)] + assert unlocked == [True, True] + assert target.getvalue() == 'pending' + streams.unwrap_stdout() + + +def test_capture_hook_can_start_and_stop_another_capture( + monkeypatch: pytest.MonkeyPatch, +) -> None: + calls: list[int] = [] + + class ReentrantStreams(utils.StreamWrapper): + reentered: bool = False + + def update_capturing(self) -> None: + calls.append(self.capturing) + if self.capturing == 0 and not self.reentered: + self.reentered = True + self.start_capturing() + self.stdout.write('nested') + self.stop_capturing() + super().update_capturing() + + streams: ReentrantStreams = ReentrantStreams() + target: io.StringIO = io.StringIO() + streams.stdout = streams.original_stdout = target + monkeypatch.setattr(sys, 'stdout', target) + wrapper: utils.WrappingIO = streams.wrap_stdout() + streams.start_capturing() + wrapper.write('outer') + streams.stop_capturing() + assert calls == [1, 0, 1, 0] + assert streams.capturing == 0 + assert target.getvalue() == 'outernested' + streams.unwrap_stdout() + + +@pytest.mark.parametrize( + 'failure_at', ['write', 'target_flush', 'buffer_flush', 'drain'] +) +@pytest.mark.parametrize('hook_raises', [False, True]) +def test_failed_stop_still_notifies_hook_and_preserves_first_error( # noqa: C901 + monkeypatch: pytest.MonkeyPatch, + failure_at: str, + hook_raises: bool, +) -> None: + failure: OSError = OSError('original stream failure') + calls: list[int] = [] + unlocked: list[bool] = [] + + class FailingTarget(io.StringIO): + armed: bool = True + + def write(self, text: str) -> int: + if failure_at == 'write' and self.armed: + self.armed = False + raise failure + return super().write(text) + + def flush(self) -> None: + if failure_at == 'target_flush' and self.armed: + self.armed = False + raise failure + super().flush() + + class FailingBuffer(io.StringIO): + armed: bool = True + + def getvalue(self) -> str: + if failure_at == 'drain' and self.armed: + self.armed = False + raise failure + return super().getvalue() + + def flush(self) -> None: + if failure_at == 'buffer_flush' and self.armed: + self.armed = False + raise failure + super().flush() + + class CustomStreams(utils.StreamWrapper): + def update_capturing(self) -> None: + calls.append(self.capturing) + + def check_locks() -> None: + for lock in (self.lock, self.stdout._buffer_box.lock): + acquired: bool = lock.acquire(timeout=1) + unlocked.append(acquired) + if acquired: + lock.release() + + observer: threading.Thread = threading.Thread(target=check_locks) + observer.start() + observer.join(3) + assert not observer.is_alive() + super().update_capturing() + if self.capturing == 0 and hook_raises: + raise RuntimeError('secondary callback failure') + + streams: CustomStreams = CustomStreams() + out: FailingTarget = FailingTarget() + err: io.StringIO = io.StringIO() + streams.stdout = streams.original_stdout = out + streams.stderr = streams.original_stderr = err + monkeypatch.setattr(sys, 'stdout', out) + monkeypatch.setattr(sys, 'stderr', err) + stdout: utils.WrappingIO = streams.wrap_stdout() + stderr: utils.WrappingIO = streams.wrap_stderr() + streams.start_capturing() + stdout.buffer = FailingBuffer() + stdout.write('spam') + stderr.write('eggs') + with pytest.raises(OSError) as excinfo: + streams.stop_capturing() + assert excinfo.value is failure + if hook_raises: + assert isinstance(failure.__context__, RuntimeError) + assert str(failure.__context__) == 'secondary callback failure' + assert calls == [1, 0] + assert unlocked == [True, True, True, True] + 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 '' + ) + streams.unwrap_stdout() + streams.unwrap_stderr() + + +def test_failed_start_state_propagation_still_notifies_hook( + monkeypatch: pytest.MonkeyPatch, +) -> None: + failure: OSError = OSError('capture flag failed') + calls: list[int] = [] + unlocked: list[bool] = [] + + class FailingWrapper(utils.WrappingIO): + armed: bool = False + + def __setattr__(self, name: str, value: typing.Any) -> None: + if name == 'capturing' and self.armed: + self.armed = False + raise failure + super().__setattr__(name, value) + + class CustomStreams(utils.StreamWrapper): + def update_capturing(self) -> None: + calls.append(self.capturing) + + def check_lock() -> None: + acquired: bool = self.lock.acquire(timeout=1) + unlocked.append(acquired) + if acquired: + self.lock.release() + + observer: threading.Thread = threading.Thread(target=check_lock) + observer.start() + observer.join(2) + assert not observer.is_alive() + super().update_capturing() + raise RuntimeError('secondary callback failure') + + streams: CustomStreams = CustomStreams() + target: io.StringIO = io.StringIO() + streams.stdout = streams.original_stdout = target + monkeypatch.setattr(sys, 'stdout', target) + streams.wrap_stdout() + wrapper: FailingWrapper = FailingWrapper(target) + streams.stdout = wrapper + monkeypatch.setattr(sys, 'stdout', wrapper) + wrapper.armed = True + with pytest.raises(OSError) as excinfo: + streams.start_capturing() + assert excinfo.value is failure + assert isinstance(failure.__context__, RuntimeError) + assert str(failure.__context__) == 'secondary callback failure' + assert calls == [1] + assert unlocked == [True] + assert streams.capturing == 1 + assert wrapper.capturing is True + streams.unwrap_stdout() + + +@pytest.mark.parametrize('failed', ['stdout', 'stderr']) +@pytest.mark.parametrize( + 'operation', ['flush', 'stop_capturing', 'excepthook'] +) +def test_drain_failure_preserves_other_stream( + failed: str, operation: str +) -> None: + streams: utils.StreamWrapper = utils.StreamWrapper() + targets: dict[str, io.StringIO] = {} + for name in ('stdout', 'stderr'): + targets[name] = io.StringIO() + wrapper: utils.WrappingIO = utils.WrappingIO( + targets[name], capturing=True + ) + setattr(streams, name, wrapper) + setattr(streams, f'wrapped_{name}', 1) + wrapper.write(name) + streams.capturing = 1 + getattr(streams, failed).buffer.close() + streams.original_excepthook = lambda *args: None + with pytest.raises(ValueError, match='closed'): + if operation == 'excepthook': + streams.excepthook(ValueError, ValueError(), None) + else: + getattr(streams, operation)() + healthy: str = 'stderr' if failed == 'stdout' else 'stdout' + assert targets[healthy].getvalue() == healthy + + +@pytest.mark.parametrize('name', ['stdout', 'stderr']) +def test_closed_buffer_unwrap_restores_stream( + name: str, monkeypatch: pytest.MonkeyPatch +) -> None: + streams: utils.StreamWrapper = utils.StreamWrapper() + original: io.StringIO = io.StringIO() + setattr(streams, f'original_{name}', original) + monkeypatch.setattr(sys, name, original) + wrapper: utils.WrappingIO = getattr(streams, f'wrap_{name}')() + wrapper.buffer.close() + try: + getattr(streams, f'unwrap_{name}')() + assert getattr(sys, name) is original + assert getattr(streams, name) is original + assert getattr(streams, f'wrapped_{name}') == 0 + finally: + setattr(streams, name, original) + setattr(streams, f'wrapped_{name}', 0) + streams.unwrap_excepthook() + + +@pytest.mark.parametrize( + 'method', ['write', 'getvalue', 'seek', 'truncate', 'flush'] +) +@pytest.mark.parametrize('instance_shadow', [False, True]) +def test_custom_buffer_methods_run_without_stream_locks( + method: str, instance_shadow: bool +) -> None: + streams: utils.StreamWrapper = utils.StreamWrapper() + target: io.StringIO = io.StringIO() + wrapper: utils.WrappingIO = utils.WrappingIO(target, capturing=True) + streams.stdout = wrapper + streams.wrapped_stdout = 1 + unlocked: list[bool] = [] + native: typing.Callable[..., typing.Any] = getattr(io.StringIO, method) + + def callback(buffer: io.StringIO, *args: typing.Any) -> typing.Any: + def observer() -> None: + for lock in (streams.lock, wrapper._buffer_box.lock): + acquired: bool = lock.acquire(timeout=0.2) + unlocked.append(acquired) + if acquired: + lock.release() + + thread: threading.Thread = threading.Thread(target=observer) + thread.start() + thread.join(2) + assert not thread.is_alive() + return native(buffer, *args) + + if instance_shadow: + wrapper.buffer = io.StringIO() + setattr( + wrapper.buffer, + method, + lambda *args: callback(wrapper.buffer, *args), + ) + else: + buffer_type: type[io.StringIO] = type( + 'CustomBuffer', (io.StringIO,), {method: callback} + ) + wrapper.buffer = buffer_type() + wrapper.write('pending') + streams.flush() + assert unlocked and all(unlocked) + assert target.getvalue() == 'pending' + + +def test_two_drain_failures_preserve_first_exception_and_context() -> None: + failures: list[BaseException] = [ + KeyboardInterrupt('first'), + RuntimeError('second'), + ] + streams: utils.StreamWrapper = utils.StreamWrapper() + for name, failure in zip(('stdout', 'stderr'), failures, strict=True): + wrapper: utils.WrappingIO = utils.WrappingIO(io.StringIO()) + + def fail(error: BaseException = failure) -> str: + raise error + + wrapper.buffer.getvalue = fail # type: ignore[method-assign] + setattr(streams, name, wrapper) + setattr(streams, f'wrapped_{name}', 1) + with pytest.raises(KeyboardInterrupt) as caught: + streams.flush() + assert caught.value is failures[0] + assert caught.value.__context__ is failures[1] + + +@pytest.mark.parametrize('name', ['stdout', 'stderr']) +def test_unwrap_restores_stream_even_when_capture_flag_raises( + name: str, monkeypatch: pytest.MonkeyPatch +) -> None: + failure: KeyboardInterrupt = KeyboardInterrupt('capture flag') + + class Wrapper(utils.WrappingIO): + armed: bool = False + + def __setattr__(self, key: str, value: typing.Any) -> None: + if key == 'capturing' and self.armed: + raise failure + super().__setattr__(key, value) + + streams: utils.StreamWrapper = utils.StreamWrapper() + original: io.StringIO = io.StringIO() + wrapper: Wrapper = Wrapper(original, capturing=True) + wrapper.armed = True + setattr(streams, name, wrapper) + setattr(streams, f'original_{name}', original) + setattr(streams, f'wrapped_{name}', 1) + monkeypatch.setattr(sys, name, wrapper) + with pytest.raises(KeyboardInterrupt) as caught: + getattr(streams, f'unwrap_{name}')() + assert caught.value is failure + assert getattr(sys, name) is original + assert getattr(streams, name) is original + assert getattr(streams, f'wrapped_{name}') == 0 + + +def test_stop_propagation_failure_notifies_hook() -> None: + failure: RuntimeError = RuntimeError('capture propagation') + notifications: list[int] = [] + + class Streams(utils.StreamWrapper): + def _apply_capturing(self) -> list[utils._Pending]: + raise failure + + def update_capturing(self) -> None: + notifications.append(self.capturing) + + streams: Streams = Streams() + streams.capturing = 1 + with pytest.raises(RuntimeError) as caught: + streams.stop_capturing() + assert caught.value is failure + assert notifications == [0] + + +@pytest.mark.parametrize('hook_error', ['distinct', 'primary', 'secondary']) +@pytest.mark.parametrize('with_context', [False, True]) +def test_capture_hook_retains_both_drain_failure_contexts( + hook_error: str, with_context: bool +) -> None: + first: KeyboardInterrupt = KeyboardInterrupt('stdout drain') + second: RuntimeError = RuntimeError('stderr drain') + third: BaseException = { + 'distinct': OSError('capture hook'), + 'primary': first, + 'secondary': second, + }[hook_error] + origins: list[BaseException] = [] + if with_context: + origins = [LookupError('first origin'), LookupError('second origin')] + first.__context__, second.__context__ = origins + if hook_error == 'distinct': + origin: LookupError = LookupError('hook origin') + third.__context__ = origin + origins.append(origin) + + class Streams(utils.StreamWrapper): + def update_capturing(self) -> None: + raise third + + streams: Streams = Streams() + for name, error in (('stdout', first), ('stderr', second)): + wrapper: utils.WrappingIO = utils.WrappingIO(io.StringIO()) + + def fail(exception: BaseException = error) -> str: + raise exception + + wrapper.buffer.getvalue = fail # type: ignore[method-assign] + setattr(streams, name, wrapper) + setattr(streams, f'wrapped_{name}', 1) + with pytest.raises(KeyboardInterrupt) as caught: + streams.stop_capturing() + assert caught.value is first + chain: list[BaseException] = [] + current: BaseException | None = caught.value + while current is not None: + assert all(current is not previous for previous in chain) + chain.append(current) + current = current.__context__ + assert second in chain + assert third in chain + assert all(origin in chain for origin in origins) + if hook_error == 'distinct': + assert first.__context__ is third diff --git a/uv.lock b/uv.lock index 11eed6ef..f9a4ed47 100644 --- a/uv.lock +++ b/uv.lock @@ -1,5 +1,5 @@ version = 1 -revision = 3 +revision = 5 requires-python = ">=3.10" resolution-markers = [ "python_full_version >= '3.15'", @@ -803,6 +803,7 @@ fast = [ tests = [ { name = "dill" }, { name = "freezegun" }, + { name = "markupsafe" }, { name = "pytest" }, { name = "pytest-cov" }, { name = "pywin32", marker = "sys_platform == 'win32'" }, @@ -818,12 +819,13 @@ requires-dist = [ { name = "dill", marker = "extra == 'tests'", specifier = ">=0.3.6" }, { name = "freezegun", marker = "extra == 'docs-tests'", specifier = ">=0.3.11" }, { name = "freezegun", marker = "extra == 'tests'", specifier = ">=0.3.11" }, + { name = "markupsafe", marker = "extra == 'tests'", specifier = ">=2.0" }, { name = "playwright", marker = "extra == 'docs-tests'", specifier = ">=1.48.0" }, { name = "pytest", marker = "extra == 'docs-tests'", specifier = ">=4.6.9" }, { name = "pytest", marker = "extra == 'tests'", specifier = ">=4.6.9" }, { name = "pytest-cov", marker = "extra == 'docs-tests'", specifier = ">=2.6.1" }, { name = "pytest-cov", marker = "extra == 'tests'", specifier = ">=2.6.1" }, - { name = "python-utils", specifier = ">=3.8.1" }, + { name = "python-utils", specifier = ">=4.1.4" }, { name = "pywin32", marker = "sys_platform == 'win32' and extra == 'tests'" }, { name = "speedups", marker = "extra == 'fast'", specifier = ">=2.1.0" }, { name = "sphinx", marker = "extra == 'docs'", specifier = ">=1.8.5" }, @@ -901,14 +903,14 @@ wheels = [ [[package]] name = "python-utils" -version = "3.9.1" +version = "4.1.4" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "typing-extensions" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/13/4c/ef8b7b1046d65c1f18ca31e5235c7d6627ca2b3f389ab1d44a74d22f5cc9/python_utils-3.9.1.tar.gz", hash = "sha256:eb574b4292415eb230f094cbf50ab5ef36e3579b8f09e9f2ba74af70891449a0", size = 35403, upload-time = "2024-11-26T00:38:58.736Z" } +sdist = { url = "https://files.pythonhosted.org/packages/c9/e7/064880019d2f205b837e5c463eb472b36e220d380e5733e49911975f7072/python_utils-4.1.4.tar.gz", hash = "sha256:a8c01f8c0b5a113fa1ab663308bb8a4773afec3cc8f9e6baeb756ac97377f8ce", size = 80130, upload-time = "2026-10-21T01:07:07.458Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/d4/69/31c82567719b34d8f6b41077732589104883771d182a9f4ff3e71430999a/python_utils-3.9.1-py2.py3-none-any.whl", hash = "sha256:0273d7363c7ad4b70999b2791d5ba6b55333d6f7a4e4c8b6b39fb82b5fab4613", size = 32078, upload-time = "2024-11-26T00:38:57.488Z" }, + { url = "https://files.pythonhosted.org/packages/3b/97/4dfe369f4e1eed1f50e7cbfaf5ef5e1fa025e8fa29073290d77d7aea89b1/python_utils-4.1.4-py3-none-any.whl", hash = "sha256:009251f6182f872cd4a74225cdf3636f48ee4596532082bd367797c5924d92bf", size = 49838, upload-time = "2026-10-21T01:07:05.761Z" }, ] [[package]]