From a480cfabf03d09a588f394a4047105f5bb17e153 Mon Sep 17 00:00:00 2001 From: Paul Saxe Date: Sat, 24 Oct 2026 03:55:39 -0400 Subject: [PATCH] Bugfix: a reference cited with new text locked the database cite() found an existing citation only by its raw text, so SEAMM's own citation, whose text holds the version, was inserted again after an update and failed on the unique alias/DOI -- leaving its transaction open, which held the database's lock: a resumed SEAMM job's parallel iterations then all failed with 'database is locked' (SEAMM job 5366). Citations are now also found by alias or DOI (their text updated), a failed citation is rolled back, and connections wait up to 30 s for a lock. Co-Authored-By: Claude Opus 5.5 --- reference_handler/reference_handler.py | 69 ++++++++++++++----- .../tests/test_updated_citations.py | 68 ++++++++++++++++++ 2 files changed, 120 insertions(+), 17 deletions(-) create mode 100644 reference_handler/tests/test_updated_citations.py diff --git a/reference_handler/reference_handler.py b/reference_handler/reference_handler.py index 2d32bbf..dd55b9e 100644 --- a/reference_handler/reference_handler.py +++ b/reference_handler/reference_handler.py @@ -159,7 +159,10 @@ def __init__(self, database): SQLite database and bulding the two tables within it. """ - self.conn = sqlite3.connect(database) + # Wait for a lock rather than failing at once: the iterations of a parallel + # loop cite into the job's one database at the same time, on a shared file + # system. + self.conn = sqlite3.connect(database, timeout=30.0) self.cur = self.conn.cursor() self._initialize_tables() @@ -359,31 +362,63 @@ def cite( doi = self._extract_doi(raw, fmt) - reference_id = self._get_reference_id(raw=raw, alias=alias, doi=doi) - - if reference_id is None: - self._create_citation(raw=raw, alias=alias, doi=doi) - reference_id = self.cur.lastrowid - self._create_context( - reference_id=reference_id, module=module, note=note, level=level - ) - else: - context_id = self._get_context_id( - reference_id=reference_id, module=module, note=note, level=level - ) + try: + reference_id = self._find_citation(raw=raw, alias=alias, doi=doi) - if context_id is None: + if reference_id is None: + self._create_citation(raw=raw, alias=alias, doi=doi) + reference_id = self.cur.lastrowid self._create_context( reference_id=reference_id, module=module, note=note, level=level ) else: - self._update_counter(context_id=context_id) + context_id = self._get_context_id( + reference_id=reference_id, module=module, note=note, level=level + ) - # Save the changes! - self.conn.commit() + if context_id is None: + self._create_context( + reference_id=reference_id, module=module, note=note, level=level + ) + else: + self._update_counter(context_id=context_id) + + # Save the changes! + self.conn.commit() + except Exception: + # Never leave the transaction open: it would hold the database's lock, + # and every other writer -- e.g. the iterations of a parallel loop -- + # would fail with "database is locked". + self.conn.rollback() + raise return reference_id + def _find_citation(self, raw=None, alias=None, doi=None): + """The id of the citation with this raw text, or else this alias or DOI. + + The same reference cited with a different text -- e.g. a program's own + citation, whose text holds its version, after an update -- is the same + citation; its text is brought up to date. Inserting it again would break + the uniqueness of the alias and DOI (a resumed job's + "UNIQUE constraint failed: citation.doi"). + """ + reference_id = self._get_reference_id(raw=raw) + if reference_id is not None: + return reference_id + for key, value in (("alias", alias), ("doi", doi)): + if value is None: + continue + self.cur.execute(f"SELECT id FROM citation WHERE {key}=?;", (value,)) + ret = self.cur.fetchall() + if len(ret) > 0: + reference_id = ret[0][0] + self.cur.execute( + "UPDATE citation SET raw=? WHERE id=?;", (raw, reference_id) + ) + return reference_id + return None + def _update_counter(self, context_id=None): """ Updates the counter for given context diff --git a/reference_handler/tests/test_updated_citations.py b/reference_handler/tests/test_updated_citations.py new file mode 100644 index 0000000..216185e --- /dev/null +++ b/reference_handler/tests/test_updated_citations.py @@ -0,0 +1,68 @@ +"""A reference cited again with a different text -- a program's citation after an +update, whose text holds its version -- is the same citation, and a failed citation +never leaves the database locked (SEAMM job 5366, 2026-10-23: a resumed job's +'UNIQUE constraint failed: citation.doi' kept the lock, and the parallel loop's +iterations then failed with 'database is locked').""" + +import sqlite3 + +import pytest + +import reference_handler + + +def raw(version): + return ( + "@misc{seamm_exec,\n" + " author = {Saxe, Paul},\n" + " title = {seamm_exec},\n" + f" version = {{{version}}},\n" + " doi = {10.5281/zenodo.0000000},\n" + "}\n" + ) + + +def cite(rh, version): + return rh.cite( + raw=raw(version), + alias="seamm_exec", + module="seamm_exec", + level=1, + note="The executor", + ) + + +def test_a_new_version_is_the_same_citation(tmp_path): + db = str(tmp_path / "references.db") + rh = reference_handler.Reference_Handler(db) + first = cite(rh, "2026.10.20") + again = cite(rh, "2026.10.23") + assert again == first + rh.cur.execute("SELECT raw FROM citation WHERE id=?;", (first,)) + assert "2026.10.23" in rh.cur.fetchone()[0] + rh.cur.execute("SELECT count FROM context WHERE reference_id=?;", (first,)) + assert rh.cur.fetchone()[0] == 2 + + +def test_a_failed_citation_does_not_keep_the_lock(tmp_path, monkeypatch): + db = str(tmp_path / "references.db") + rh = reference_handler.Reference_Handler(db) + cite(rh, "2026.10.20") + + def broken(**kwargs): + rh.cur.execute( + "INSERT INTO citation (raw, alias, doi) VALUES (?, ?, ?);", + ("@misc{x, title={x}}", "seamm_exec", None), + ) + + monkeypatch.setattr(rh, "_find_citation", lambda **kwargs: None) + monkeypatch.setattr(rh, "_create_citation", broken) + with pytest.raises(sqlite3.IntegrityError): + cite(rh, "2026.10.23") + assert not rh.conn.in_transaction + + # Another connection -- another process of the job -- can still write + other = sqlite3.connect(db, timeout=0.5) + other.execute("UPDATE context SET count = count + 1;") + other.commit() + other.close()