Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,7 @@ Patrick Hayes
Patrick Lannigan
Paul Müller
Paul Reece
Paul Zuradzki
Pauli Virtanen
Pavel Karateev
Pavel Zhukov
Expand Down
1 change: 1 addition & 0 deletions changelog/14263.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Unraisable exceptions from finalizers are now collected during ``pytest_unconfigure``, before pytest tears down the warning filters installed for the session. Previously the collection ran from a cleanup callback whose order relative to other plugins' cleanups was not guaranteed, so an active ``error`` filter could be removed before the exception surfaced and a late resource leak would pass silently. A ``-W error`` filter, or any filter matching :class:`pytest.PytestUnraisableExceptionWarning`, now promotes these exceptions to failures regardless of plugin cleanup order.
20 changes: 20 additions & 0 deletions src/_pytest/unraisableexception.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,26 @@ def pytest_configure(config: Config) -> None:
sys.unraisablehook = functools.partial(unraisable_hook, append=deque.append)


def pytest_unconfigure(config: Config) -> None:
# Runs before ``_cleanup_stack.close()``, so warning filters from
# cleanup-stack-managed contexts (notably the ``warnings`` plugin's
# ``catch_warnings``) are still installed when garbage-collected
# finalizers fire. A ``config.add_cleanup`` callback would instead
# couple correctness to LIFO pop order across plugins' cleanups.
if unraisable_exceptions not in config.stash:
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug made possible by -- this PR -- moving GC and collect_unraisable from cleanup callback into pytest_unconfigure hook.

Guard checks: if our configure never ran, stash key is not there, and there's nothing to drain deque, so early return.

Else, this bug was possile:

  • configure step crashes before def pytest_configure(config): ... config.stash[unraisable_exceptions] = deque
  • stash never created
  • unconfigure runs anyway
  • -> tries to read missing stash
  • -> KeyError

# ``pytest_configure`` did not complete (e.g. a usage error raised
# in another plugin's configure), so the queue stash was never set.
return
# PyPy can resurrect objects in __del__, so it needs several GC passes
# (5, per the Trio project); CPython frees cycles in one pass. See #14441.
_default_gc_collect_iterations = 5 if sys.implementation.name == "pypy" else 1
gc_collect_iterations = config.stash.get(
gc_collect_iterations_key, _default_gc_collect_iterations
)
gc_collect_harder(gc_collect_iterations)
collect_unraisable(config)


@pytest.hookimpl(trylast=True)
def pytest_runtest_setup(item: Item) -> None:
collect_unraisable(item.config)
Expand Down
134 changes: 134 additions & 0 deletions testing/test_unraisableexception.py
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,8 @@ def test_scheduler_must_be_created_within_running_loop() -> None:

# TODO: Should be a test failure or error. Currently the exception
# propagates all the way to the top resulting in exit code 1.
# -Werror matches the wrapping PytestUnraisableExceptionWarning, so the
# wrapper propagates; its message embeds the original RuntimeWarning.
assert result.ret == 1

result.assert_outcomes(passed=1)
Expand Down Expand Up @@ -359,6 +361,138 @@ def test_it():
result.stderr.fnmatch_lines("ValueError: del is broken")


def test_unraisable_warning_without_filter_still_wraps(pytester: Pytester) -> None:
# A Warning raised from ``__del__`` gets wrapped in
# PytestUnraisableExceptionWarning like any other unraisable exception.
# pytest does not unwrap it to honor a filter on the inner class, so with
# no filter matching the wrapper, pytest logs the warning and the run
# passes. This guards against re-introducing the dropped unwrap path.
pytester.makepyfile(
test_it="""
class RaisingDel:
def __del__(self):
raise UserWarning("oops")

def test_it():
obj = RaisingDel()
del obj
"""
)

# Subprocess so we don't inherit the outer pytest's ``error`` filter from
# pyproject.toml, which would promote the wrapper and fail the run.
# ``-Wdefault::pytest.PytestUnraisableExceptionWarning`` makes the wrapping
# warning visible on stderr whether ``__del__`` fires inside the test or
# during later GC (PyPy).
result = pytester.runpytest_subprocess(
"-Wdefault::pytest.PytestUnraisableExceptionWarning"
)

assert result.ret == 0
result.assert_outcomes(passed=1)
# The unraisable hook emitted PytestUnraisableExceptionWarning.
# The warning lands in the warnings-summary section of stdout on
# CPython (where ``__del__`` fires inside the test) and on stderr on
# PyPy (where it fires during later GC). Check both rather than the
# outcomes counter, which is timing-dependent.
combined = "\n".join(result.outlines + result.errlines)
assert "PytestUnraisableExceptionWarning" in combined


@pytest.mark.skipif(PYPY, reason="garbage-collection differences make this flaky")
def test_unraisable_decouples_from_cleanup_stack_order(pytester: Pytester) -> None:
# Regression test for the structural fix. The garbage-collection step
# that surfaces queued unraisables must run before _cleanup_stack.close()
# so warning filters installed via cleanup-stack-managed contexts are
# still in effect when finalizers fire. Otherwise correctness depends
# on the order in which plugins register their cleanups under LIFO.
#
# The conftest uses ``@hookimpl(trylast=True)`` so its pytest_configure
# runs after all built-in pytest_configures. Its
# ``warnings.resetwarnings`` cleanup then lands on the cleanup stack
# last and pops first under LIFO. Under the pre-fix layout, where
# garbage collection runs inside unraisableexception's own cleanup
# callback, that pop clears the user's
# ``error::pytest.PytestUnraisableExceptionWarning`` filter before the
# finalizer raises; pytest logs the wrapped warning instead of
# promoting it, and the suite exits 0. Under the post-fix layout,
# ``pytest_unconfigure`` runs the garbage collection and queue processing
# before the cleanup stack starts closing, so the conftest's reset has no
# effect.
pytester.makeini(
"""
[pytest]
filterwarnings =
error::pytest.PytestUnraisableExceptionWarning
"""
)
pytester.makeconftest(
"""
import warnings
import pytest

@pytest.hookimpl(trylast=True)
def pytest_configure(config):
config.add_cleanup(warnings.resetwarnings)
"""
)
pytester.makepyfile(
test_it="""
import gc; gc.disable()

class BrokenDel:
def __init__(self):
self.self = self # cycle survives until session-end gc

def __del__(self):
raise ValueError("del is broken")

def test_it():
BrokenDel()
"""
)

result = pytester.runpytest_subprocess()

assert result.ret == 1, (
"wrapper filter is still installed at GC time, so the unraisable "
"exits non-zero regardless of cleanup-stack pop order"
)
result.assert_outcomes(passed=1)
result.stderr.fnmatch_lines("*ValueError: del is broken*")


def test_pytest_unconfigure_survives_failed_pytest_configure(
pytester: Pytester,
) -> None:
# Regression test for the guard against an unset stash key. When
# another plugin's pytest_configure raises pytest.UsageError, pluggy
# stops calling the remaining configure hooks; the unraisable plugin's
# pytest_configure never runs and config.stash[unraisable_exceptions]
# stays unset. pytest_unconfigure still fires for partially-configured
# runs, so without a presence check the unraisable plugin's
# pytest_unconfigure raises KeyError and pytest reports INTERNALERROR
# instead of USAGE_ERROR.
pytester.makeconftest(
"""
import pytest

def pytest_configure(config):
raise pytest.UsageError("simulated bad config")
"""
)
pytester.makepyfile(test_it="def test_it(): pass")

result = pytester.runpytest_subprocess()

assert result.ret == pytest.ExitCode.USAGE_ERROR, (
"the UsageError must surface as USAGE_ERROR, not a KeyError INTERNALERROR"
)
result.stderr.fnmatch_lines("*ERROR: simulated bad config*")
result.stderr.no_fnmatch_line("*INTERNALERROR*")
result.stderr.no_fnmatch_line("*KeyError*")


@pytest.mark.filterwarnings("error::pytest.PytestUnraisableExceptionWarning")
def test_possibly_none_excinfo(pytester: Pytester) -> None:
pytester.makepyfile(
Expand Down
Loading