From 79fe04c6aca32174c228095e56c8def2d1688667 Mon Sep 17 00:00:00 2001 From: william Date: Tue, 25 Aug 2026 15:06:16 +0200 Subject: [PATCH] =?UTF-8?q?Corrige=20"database=20is=20locked"=20caus=C3=A9?= =?UTF-8?q?=20par=20une=20connexion=20SQLite=20qui=20fuit=20apr=C3=A8s=20u?= =?UTF-8?q?n=20plantage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug remonté : suppression d'un élément échouant avec sqlite3.OperationalError: database is locked, exactement sur le conn.execute() de delete_element.py. La trace complète montrait que la connexion attendait puis lâchait après le timeout (10s) - pas une simple collision passagère entre deux requêtes (déjà gérée par WAL + busy_timeout, voir les commentaires existants de connect()), mais un verrou tenu bien plus longtemps : une connexion ouverte par une requête ANTÉRIEURE qui a planté, jamais fermée. Cause de fond : chaque fonction de db/ (~80 d'entre elles) ouvre sa propre connexion et est censée la fermer elle-même avant de rendre la main - si une exception survient entre l'ouverture et cette fermeture, le conn.close() prévu n'est jamais atteint. En mode debug (voir app.py), le débogueur Werkzeug garde la trace complète de l'erreur en mémoire pour l'inspection interactive, ce qui inclut la variable locale `conn` : empêchée d'être ramassée par le GC, elle ne libère jamais son verrou d'écriture SQLite - bloquant TOUTE écriture suivante jusqu'au redémarrage du serveur, même longtemps après l'erreur d'origine et sans lien apparent avec elle (d'où la confusion : l'erreur semble venir de l'action qui échoue, alors qu'elle est victime d'une fuite antérieure). Fix, dans db/connection.py, sans toucher aux ~80 fonctions existantes : connect() enregistre maintenant chaque connexion sur le contexte de la requête Flask en cours (flask.g, uniquement quand il y en a un - un appel direct hors requête, scripts/tests, n'est pas concerné) ; un teardown_request ferme toute connexion encore ouverte à la fin de CHAQUE requête, qu'elle ait réussi ou planté (garanti par Flask, contrairement à after_request). Fermer une connexion déjà fermée normalement ne fait rien, donc aucun changement de comportement pour le cas normal. Ajoute tests/test_db_connection_leak_safety_net.py, qui reproduit le scénario exact (connexion ouverte puis exception avant fermeture) et vérifie qu'une écriture suivante ne bloque plus - désactivé temporairement pour confirmer que le test échoue bien (et de la même façon) sans le fix. Co-Authored-By: Claude Sonnet 5 --- db/connection.py | 52 +++++++++++++++++++++ tests/test_db_connection_leak_safety_net.py | 43 +++++++++++++++++ 2 files changed, 95 insertions(+) create mode 100644 tests/test_db_connection_leak_safety_net.py diff --git a/db/connection.py b/db/connection.py index 5eff1c3a..21805dae 100644 --- a/db/connection.py +++ b/db/connection.py @@ -17,4 +17,56 @@ def connect(slug): conn.execute("PRAGMA foreign_keys = ON") conn.execute("PRAGMA journal_mode = WAL") conn.execute("PRAGMA busy_timeout = 8000") + _track_for_teardown(conn) return conn + + +def _track_for_teardown(conn): + """Filet de sécurité : chaque fonction de db/ ouvre sa propre connexion + et est censée la fermer elle-même (conn.close()) avant de rendre la + main — mais si une exception survient ENTRE l'ouverture et cette + fermeture (une erreur de programmation, une contrainte violée...), le + conn.close() prévu n'est jamais atteint. En mode debug (voir app.py), + le débogueur Werkzeug garde alors la trace complète de l'erreur en + mémoire pour l'inspection interactive — ce qui inclut la variable + locale `conn`, empêchant le ramasse-miettes Python de la libérer et + donc SQLite de relâcher son verrou d'écriture. Toute requête suivante + qui écrit se heurte alors à "database is locked" jusqu'au redémarrage + du serveur, même longtemps après l'erreur d'origine. En enregistrant + ici la connexion sur le contexte de la requête Flask en cours (quand il + y en a un), on garantit sa fermeture à la fin de la requête via + _close_leaked_connections ci-dessous, que la requête ait réussi ou + planté — sans rien changer au comportement des ~80 fonctions qui + ferment déjà correctement leur connexion (fermer une connexion SQLite + déjà fermée ne fait rien).""" + try: + from flask import g, has_app_context + except ImportError: + return + if not has_app_context(): + return + if not hasattr(g, "_forge_db_connections"): + g._forge_db_connections = [] + g._forge_db_connections.append(conn) + + +def _install_teardown_safety_net(): + """Appelé une seule fois (voir le bas de ce fichier) — enregistre le + filet de sécurité sur l'appli Flask. `core.flask_app` ne dépend de rien + dans `db/`, donc cet import ne crée pas de dépendance circulaire.""" + try: + from core.flask_app import app + except ImportError: + return + + @app.teardown_request + def _close_leaked_connections(exception=None): # noqa: ARG001 - signature imposée par Flask + from flask import g + for conn in getattr(g, "_forge_db_connections", ()): + try: + conn.close() + except sqlite3.Error: + pass + + +_install_teardown_safety_net() diff --git a/tests/test_db_connection_leak_safety_net.py b/tests/test_db_connection_leak_safety_net.py new file mode 100644 index 00000000..6cfc6065 --- /dev/null +++ b/tests/test_db_connection_leak_safety_net.py @@ -0,0 +1,43 @@ +"""Régression : une connexion SQLite ouverte puis jamais fermée à cause +d'une exception ne doit plus bloquer les écritures suivantes avec +"database is locked" — voir _track_for_teardown()/_install_teardown_ +safety_net() dans db/connection.py, qui ferme toute connexion encore +ouverte à la fin de la requête, que celle-ci ait réussi ou planté.""" +import db +import screens + +from core.flask_app import app + + +def test_connection_left_open_by_a_crashed_request_is_closed_at_teardown(game): + """Simule exactement l'incident remonté : du code qui ouvre une + connexion en écriture puis plante avant de la fermer (comme + delete_element.py le ferait si conn.execute() levait une exception), + à l'intérieur d'une requête Flask (test_request_context — quitter le + "with" déclenche le même teardown_request qu'une vraie requête, qu'elle + ait réussi ou non).""" + screens.list_screens(game) # garantit que _screens existe déjà (ensure_schema) + try: + with app.test_request_context(f"/__test_leak/{game}"): + conn = db.connect(game) + conn.execute("INSERT INTO _screens (name, is_template) VALUES ('x', 0)") + raise RuntimeError("simulate a crash before conn.commit()/conn.close()") + except RuntimeError: + pass + + # Si la connexion précédente n'avait pas été fermée par le filet de + # sécurité, cette écriture échouerait avec sqlite3.OperationalError: + # database is locked (timeout=10s côté connect(), donc le test + # planterait/traînerait plutôt que de simplement échouer vite). + screen_id = screens.create_screen(game, "Un écran normal") + assert screen_id + + +def test_connection_is_not_tracked_outside_a_request_context(game): + """Non-régression : un appel direct à db.connect() en dehors de tout + contexte Flask (scripts, tests) ne doit pas planter — has_app_context() + doit simplement renvoyer False et ne rien suivre.""" + with app.app_context(): + pass # contexte créé puis immédiatement fermé, comme hors requête + conn = db.connect(game) + conn.close()