Claude commited on
Commit
e7041cb
·
unverified ·
1 Parent(s): d0a3fab

fix(ci+storage): unblock Python 3.12 shutdown hang on ubuntu

Browse files

Symptôme CI Python 3.12 ubuntu :
pytest finit en 3:21 (5013 passed)
interpréteur reste 12 min en hang
runner GitHub timeout 15 min → "action timed out"

Cause racine
============

picarones/adapters/storage/job_store.py utilisait le pattern :

with self._connect() as conn:
conn.execute(...)

Or le __exit__ de sqlite3.Connection ne fait QU'UN COMMIT,
**pas un close()**. Chaque appel à une méthode du JobStore créait
une nouvelle connexion qui n'était jamais fermée explicitement.

Sur Python 3.12+ le GC de fin d'interpréteur tente de fermer les
milliers de connexions accumulées (5013 tests × N opérations),
ce qui peut bloquer plusieurs minutes — observable d'autant plus
clairement que 3.12 a renforcé le warning ResourceWarning sur les
ressources non fermées (compte de warnings : 51 sur 3.11 → 85 sur
3.12 → 341 sur 3.13).

Sur 3.11 le shutdown finissait dans la fenêtre du timeout-minutes
de 15 ; sur 3.12 ubuntu il dépassait.

Fix
===

1. JobStore._connect est un @contextlib.contextmanager qui
yield la connexion puis conn.close() en finally. Plus
de fuite, plus de hang au shutdown.

2. CI durci avec garde-fous anti-hang :

- timeout-minutes: 12 au step Run tests (était 15).
- timeout GNU autour de pytest : SIGTERM à 9 min,
SIGKILL 30s après si Python n'obéit pas. Le shutdown
ne peut plus monopoliser le job.
- python -X faulthandler + PYTHONFAULTHANDLER=1 :
dump des stack traces de tous les threads avant SIGKILL —
diagnostic immédiat si un nouveau hang apparaît.
- Détection timeout (Linux GNU) / gtimeout (macOS
Homebrew) / fallback Python direct (Windows).

3. tests/conftest.py : pytest_sessionfinish enumère les
threads vivants en fin de session et programme un
faulthandler.dump_traceback_later(60) qui dumpera les
stack traces si l'interpréteur ne sort pas dans la minute.

Vérification locale
===================

Tests : 5016 passed, 12 skipped, 8 deselected, 0 failed.
Lint : ruff check picarones/ tests/ clean.
La fuite n'apparaissait pas en pytest local sur 3.11 mais le fix
est sain sur toutes les versions Python.

.github/workflows/ci.yml CHANGED
@@ -92,22 +92,55 @@ jobs:
92
  # ── Tests ───────────────────────────────────────────────────
93
  # Sprint A1 : --cov-fail-under=85 (baseline mesuré 87 %, marge 2 pts).
94
  # pytest-timeout est configuré dans pyproject.toml [tool.pytest.ini_options].
95
- # ``timeout-minutes`` au niveau step : le job ne hang JAMAIS plus de
96
- # 15 min sur les tests, même si pytest-timeout (par-test) échoue à
97
- # cleanup un thread daemon.
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
98
  - name: Run tests
99
  # Sur Python 3.13, on continue malgré une erreur pour ne pas bloquer
100
  # le merge pendant la fenêtre informationnelle de 6 mois (m-8).
101
  continue-on-error: ${{ matrix.python-version == '3.13' }}
102
- timeout-minutes: 15
103
  shell: bash
104
  run: |
105
- pytest tests/ -q --tb=short --no-header \
106
- --cov=picarones --cov-report=xml --cov-report=term-missing \
107
- --cov-fail-under=85
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
108
  env:
109
  PYTHONIOENCODING: utf-8
110
  PYTHONUTF8: "1"
 
111
 
112
  # ── Couverture ──────────────────────────────────────────────
113
  # Conditions :
 
92
  # ── Tests ───────────────────────────────────────────────────
93
  # Sprint A1 : --cov-fail-under=85 (baseline mesuré 87 %, marge 2 pts).
94
  # pytest-timeout est configuré dans pyproject.toml [tool.pytest.ini_options].
95
+ #
96
+ # Garde-fous anti-hang :
97
+ #
98
+ # 1. ``timeout-minutes: 12`` au niveau step : cap dur GitHub si
99
+ # tout le reste échoue.
100
+ # 2. ``timeout`` GNU autour de pytest : SIGTERM à 9 minutes,
101
+ # SIGKILL 30s après si Python n'a pas obéi. Couvre
102
+ # spécifiquement le cas d'un hang de SHUTDOWN de
103
+ # l'interpréteur Python 3.12+ (threads non-daemon, connexions
104
+ # sqlite non fermées, ResourceWarnings — observé sur ubuntu
105
+ # 3.12 où pytest finit en 3:21 et l'interpréteur reste 12 min
106
+ # avant de rendre la main).
107
+ # 3. ``-X faulthandler`` : si le hang revient, on aura les stack
108
+ # traces de tous les threads dans le log avant le SIGKILL.
109
+ # 4. ``PYTHONFAULTHANDLER=1`` redondance ceinture-bretelles.
110
+ #
111
+ # Le code de retour 124 (SIGTERM par GNU timeout) ou 137 (SIGKILL)
112
+ # est traité comme un échec normal — on perd l'info pytest mais
113
+ # on préserve la latence de la CI.
114
  - name: Run tests
115
  # Sur Python 3.13, on continue malgré une erreur pour ne pas bloquer
116
  # le merge pendant la fenêtre informationnelle de 6 mois (m-8).
117
  continue-on-error: ${{ matrix.python-version == '3.13' }}
118
+ timeout-minutes: 12
119
  shell: bash
120
  run: |
121
+ # ``timeout`` n'est pas standard sur macOS (BSD vs GNU) — on
122
+ # détecte et on adapte. Sur Windows, le shell bash de
123
+ # Git-Bash n'a pas timeout : on retombe sur python direct.
124
+ if command -v timeout >/dev/null 2>&1; then
125
+ timeout --signal=SIGTERM --kill-after=30 540 \
126
+ python -X faulthandler -m pytest tests/ -q --tb=short --no-header \
127
+ --cov=picarones --cov-report=xml --cov-report=term-missing \
128
+ --cov-fail-under=85
129
+ elif command -v gtimeout >/dev/null 2>&1; then
130
+ # macOS Homebrew coreutils.
131
+ gtimeout --signal=SIGTERM --kill-after=30 540 \
132
+ python -X faulthandler -m pytest tests/ -q --tb=short --no-header \
133
+ --cov=picarones --cov-report=xml --cov-report=term-missing \
134
+ --cov-fail-under=85
135
+ else
136
+ python -X faulthandler -m pytest tests/ -q --tb=short --no-header \
137
+ --cov=picarones --cov-report=xml --cov-report=term-missing \
138
+ --cov-fail-under=85
139
+ fi
140
  env:
141
  PYTHONIOENCODING: utf-8
142
  PYTHONUTF8: "1"
143
+ PYTHONFAULTHANDLER: "1"
144
 
145
  # ── Couverture ──────────────────────────────────────────────
146
  # Conditions :
picarones/adapters/storage/job_store.py CHANGED
@@ -59,6 +59,7 @@ from __future__ import annotations
59
 
60
  import json
61
  import logging
 
62
  import sqlite3
63
  import time
64
  from collections.abc import Callable
@@ -248,23 +249,37 @@ class JobStore:
248
  def db_path(self) -> Path:
249
  return self._path
250
 
251
- def _connect(self) -> sqlite3.Connection:
252
- """Ouvre une nouvelle connexion.
 
253
 
254
  ``timeout=30s`` côté driver Python + ``PRAGMA busy_timeout``
255
  côté SQLite absorbent les contentions courtes. Le mode
256
  autocommit combiné au journal WAL garantit que les lectures
257
  n'attendent pas les écritures (cf. https://sqlite.org/wal.html).
 
 
 
 
 
 
 
 
 
 
258
  """
259
  conn = sqlite3.connect(
260
  str(self._path),
261
  isolation_level=None, # autocommit pour simplicité
262
  timeout=30.0,
263
  )
264
- # busy_timeout (ms) — backup au timeout Python.
265
- conn.execute("PRAGMA busy_timeout = 30000;")
266
- conn.row_factory = sqlite3.Row
267
- return conn
 
 
 
268
 
269
  # ──────────────────────────────────────────────────────────────
270
  # Création / lecture
 
59
 
60
  import json
61
  import logging
62
+ import contextlib
63
  import sqlite3
64
  import time
65
  from collections.abc import Callable
 
249
  def db_path(self) -> Path:
250
  return self._path
251
 
252
+ @contextlib.contextmanager
253
+ def _connect(self):
254
+ """Ouvre puis ferme une connexion SQLite.
255
 
256
  ``timeout=30s`` côté driver Python + ``PRAGMA busy_timeout``
257
  côté SQLite absorbent les contentions courtes. Le mode
258
  autocommit combiné au journal WAL garantit que les lectures
259
  n'attendent pas les écritures (cf. https://sqlite.org/wal.html).
260
+
261
+ Pourquoi un contextmanager dédié plutôt que ``with
262
+ sqlite3.connect(...)`` directement : le ``__exit__`` de
263
+ ``sqlite3.Connection`` fait UN COMMIT, pas un ``close()``.
264
+ Sur Python 3.12+, les connexions non fermées s'accumulent et
265
+ leur libération via GC au shutdown de l'interpréteur peut
266
+ bloquer le process plusieurs minutes (observé sur ubuntu
267
+ 3.12 — pytest finit en 3:21, l'interpréteur reste 12 min en
268
+ hang avant SIGKILL du runner CI). ``yield + close()`` dans
269
+ ``finally`` garantit la libération immédiate.
270
  """
271
  conn = sqlite3.connect(
272
  str(self._path),
273
  isolation_level=None, # autocommit pour simplicité
274
  timeout=30.0,
275
  )
276
+ try:
277
+ # busy_timeout (ms) — backup au timeout Python.
278
+ conn.execute("PRAGMA busy_timeout = 30000;")
279
+ conn.row_factory = sqlite3.Row
280
+ yield conn
281
+ finally:
282
+ conn.close()
283
 
284
  # ──────────────────────────────────────────────────────────────
285
  # Création / lecture
tests/conftest.py CHANGED
@@ -47,3 +47,51 @@ os.environ.pop("PICARONES_PUBLIC_MODE", None)
47
 
48
  # Rate limit désactivé en dev (déjà le défaut, explicité ici).
49
  os.environ.setdefault("PICARONES_RATE_LIMIT_PER_HOUR", "0")
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
47
 
48
  # Rate limit désactivé en dev (déjà le défaut, explicité ici).
49
  os.environ.setdefault("PICARONES_RATE_LIMIT_PER_HOUR", "0")
50
+
51
+
52
+ def pytest_sessionfinish(session, exitstatus) -> None: # noqa: ARG001
53
+ """Diagnostic du shutdown de l'interpréteur.
54
+
55
+ Sur Python 3.12 ubuntu-latest, l'interpréteur restait jusqu'à 12
56
+ minutes en hang après ``=== passed ===`` à cause de threads
57
+ non-daemon ou de connexions sqlite non fermées que les tests
58
+ avaient laissés.
59
+
60
+ Ce hook :
61
+
62
+ 1. Liste les threads vivants à la fin de la session — si la
63
+ liste contient autre chose que ``MainThread``, le développeur
64
+ voit immédiatement quelle ressource fuit.
65
+ 2. Force le flush stdout/stderr pour que le diagnostic apparaisse
66
+ même si l'interpréteur hang ensuite.
67
+ 3. Programme un ``faulthandler.dump_traceback_later(60)`` qui
68
+ dumpera les stack traces de TOUS les threads après 60s
69
+ d'inactivité — ce qu'on a besoin pour identifier la fuite si
70
+ le hang persiste.
71
+ """
72
+ import faulthandler
73
+ import sys
74
+ import threading
75
+
76
+ alive = [
77
+ t for t in threading.enumerate()
78
+ if t is not threading.main_thread() and t.is_alive()
79
+ ]
80
+ if alive:
81
+ sys.stderr.write(
82
+ "\n[conftest] threads encore vivants au sessionfinish "
83
+ f"({len(alive)}) :\n",
84
+ )
85
+ for t in alive:
86
+ sys.stderr.write(
87
+ f" - name={t.name!r} daemon={t.daemon} "
88
+ f"alive={t.is_alive()}\n",
89
+ )
90
+ sys.stderr.flush()
91
+
92
+ # Si le shutdown hang plus de 60s, on aura les stack traces.
93
+ faulthandler.dump_traceback_later(
94
+ timeout=60,
95
+ repeat=False,
96
+ file=sys.stderr,
97
+ )