Skip to content

Commit 9581909

Browse files
committed
[build] Fix flake due to suspected finalizer exceptions
1 parent c1fcb79 commit 9581909

5 files changed

Lines changed: 77 additions & 36 deletions

File tree

.github/workflows/build.yml

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ jobs:
3333
build:
3434
runs-on: ${{ matrix.os }}
3535
strategy:
36+
fail-fast: false
3637
matrix:
3738
os: [macos-14, macos-latest, ubuntu-22.04, ubuntu-latest, windows-latest]
3839
python-version: ["3.10", "3.11", "3.12", "3.13"]
@@ -64,9 +65,18 @@ jobs:
6465
git fetch --depth=1 https://github.com/Breakthrough/PySceneDetect.git refs/heads/resources:refs/remotes/origin/resources
6566
git checkout refs/remotes/origin/resources -- tests/resources/
6667
68+
# Instrumented while chasing a windows-latest flake where python exits 1 after a
69+
# fully green pytest run with no output. `-X dev` makes shutdown-time warnings and
70+
# finalizer errors loud; echoing the exit code separates python's own return value
71+
# from anything the shell wrapper does to a crash code.
6772
- name: Unit Tests
73+
shell: bash
6874
run: |
69-
python -m pytest -vv
75+
set +e
76+
python -X dev -m pytest -vv
77+
code=$?
78+
echo "pytest exit code: $code"
79+
exit $code
7080
7181
- name: Smoke Test (Module)
7282
run: |

pyproject.toml

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,6 @@ markers = [
9494
addopts = "-m 'not release'"
9595
filterwarnings = [
9696
"ignore:TODO.*Update caller to handle VFR:UserWarning",
97-
# Promote unraisable (finalizer) and unhandled-thread exceptions to named test
98-
# failures. A CI run on windows-latest once exited 1 with every test passing and
99-
# no output - the failure mode pytest 9.1+ reports for unraisable exceptions
100-
# collected at shutdown. These filters turn any recurrence into an attributable
101-
# traceback instead of a bare exit code.
102-
"error::pytest.PytestUnraisableExceptionWarning",
103-
"error::pytest.PytestUnhandledThreadExceptionWarning",
10497
]
10598

10699
[tool.ruff]

scenedetect/backends/pyav.py

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,10 +157,19 @@ def __init__(
157157
self._duration_frames = self._get_duration()
158158

159159
def __del__(self):
160-
# `_container` is unset if `__init__` raised before `av.open()` succeeded.
161-
container = getattr(self, "_container", None)
162-
if container is not None:
163-
container.close()
160+
# Finalizers must never raise - an exception here becomes an unraisable error. During
161+
# interpreter shutdown the underlying handles are reclaimed by the OS anyway.
162+
try:
163+
# Close the decode generator first to break its cycle with the container.
164+
decoder = getattr(self, "_decoder", None)
165+
if decoder is not None:
166+
decoder.close()
167+
# `_container` is unset if `__init__` raised before `av.open()` succeeded.
168+
container = getattr(self, "_container", None)
169+
if container is not None:
170+
container.close()
171+
except Exception:
172+
pass
164173

165174
#
166175
# VideoStream Methods/Properties

scenedetect/scene_manager.py

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -573,31 +573,37 @@ def detect_scenes(
573573
frame_im = None
574574

575575
logger.info("Detecting scenes...")
576-
while not self._stop.is_set():
577-
next_frame, position = frame_queue.get()
578-
if next_frame is None and position is None:
579-
break
580-
if next_frame is not None:
581-
frame_im = next_frame
582-
assert frame_im is not None
583-
new_cuts = self._process_frame(position, frame_im, callback)
576+
try:
577+
while not self._stop.is_set():
578+
next_frame, position = frame_queue.get()
579+
if next_frame is None and position is None:
580+
break
581+
if next_frame is not None:
582+
frame_im = next_frame
583+
assert frame_im is not None
584+
new_cuts = self._process_frame(position, frame_im, callback)
585+
if progress_bar is not None:
586+
if new_cuts:
587+
progress_bar.set_description(
588+
PROGRESS_BAR_DESCRIPTION % len(self._cutting_list), refresh=False
589+
)
590+
progress_bar.update(1 + frame_skip)
591+
finally:
584592
if progress_bar is not None:
585-
if new_cuts:
586-
progress_bar.set_description(
587-
PROGRESS_BAR_DESCRIPTION % len(self._cutting_list), refresh=False
588-
)
589-
progress_bar.update(1 + frame_skip)
590-
591-
if progress_bar is not None:
592-
progress_bar.set_description(
593-
PROGRESS_BAR_DESCRIPTION % len(self._cutting_list), refresh=True
594-
)
595-
progress_bar.close()
596-
# Unblock any puts in the decode thread before joining. This can happen if the main
597-
# processing thread stops before the decode thread.
598-
while not frame_queue.empty():
599-
frame_queue.get_nowait()
600-
decode_thread.join()
593+
progress_bar.set_description(
594+
PROGRESS_BAR_DESCRIPTION % len(self._cutting_list), refresh=True
595+
)
596+
progress_bar.close()
597+
# The decode thread must never be abandoned, even if a detector or callback
598+
# raises above: an orphaned daemon thread keeps the VideoStream alive until
599+
# interpreter shutdown, where finalizing it (or killing the thread mid-decode)
600+
# can crash process exit. Signal it to stop, then keep unblocking any pending
601+
# puts until it exits.
602+
self._stop.set()
603+
while decode_thread.is_alive():
604+
while not frame_queue.empty():
605+
frame_queue.get_nowait()
606+
decode_thread.join(timeout=0.1)
601607

602608
if self._exception_info is not None:
603609
exc = self._exception_info[1]

tests/conftest.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,3 +159,26 @@ def delayed_start_video() -> str:
159159
"""Video with a nonzero stream start time (1.075s edit-list offset). Created from
160160
fades.mp4 via: ffmpeg -itsoffset 1.075 -i fades.mp4 -t 2 -c:v copy -an delayed_start.mp4"""
161161
return check_exists("tests/resources/delayed_start.mp4")
162+
163+
164+
def pytest_unconfigure(config):
165+
"""Diagnostic for a windows-latest CI flake (silent exit 1 after a green run):
166+
report any non-main threads still alive at session end. Leaked threads keep
167+
VideoStreams alive into interpreter shutdown, where native teardown can crash
168+
the process exit code. tqdm's global monitor singleton is expected and ignored."""
169+
import sys
170+
import threading
171+
172+
leftover = [
173+
t
174+
for t in threading.enumerate()
175+
if t is not threading.main_thread() and t.name != "tqdm_monitor"
176+
]
177+
for thread in leftover:
178+
frame = sys._current_frames().get(thread.ident) if thread.ident else None
179+
location = f"{frame.f_code.co_filename}:{frame.f_lineno}" if frame else "unknown"
180+
print(
181+
f"WARNING: thread still alive at exit: {thread.name} "
182+
f"(daemon={thread.daemon}) at {location}",
183+
file=sys.stderr,
184+
)

0 commit comments

Comments
 (0)