|
10 | 10 | from the sentinel onward. If the sentinel is missing it's appended at the |
11 | 11 | end of the file. This means hand-curated preambles and seed bullets above |
12 | 12 | the sentinel survive every render. |
| 13 | +
|
| 14 | +Concurrency: append_lesson and render_lessons both acquire an advisory |
| 15 | +exclusive flock on lessons.jsonl so a concurrent appender can't land a new |
| 16 | +row between render's load and write, leaving LESSONS.md stale. LESSONS.md |
| 17 | +is rewritten atomically (temp file + rename) so readers never see a |
| 18 | +half-written file. Windows (no fcntl) falls through without locking; safe |
| 19 | +for single-user, noted in a one-time warning. |
13 | 20 | """ |
14 | | -import os, json, datetime, hashlib |
| 21 | +import os, json, datetime, hashlib, warnings |
15 | 22 | from collections import defaultdict |
| 23 | +from contextlib import contextmanager |
16 | 24 |
|
17 | 25 |
|
18 | 26 | LESSONS_JSONL = "lessons.jsonl" |
|
21 | 29 | SENTINEL = "## Auto-promoted entries will be appended below" |
22 | 30 |
|
23 | 31 |
|
| 32 | +try: |
| 33 | + import fcntl |
| 34 | + _HAS_FLOCK = True |
| 35 | +except ImportError: |
| 36 | + _HAS_FLOCK = False |
| 37 | + warnings.warn( |
| 38 | + "fcntl unavailable; lessons.jsonl concurrent-write protection " |
| 39 | + "disabled. Safe for single-user repos; not safe for shared/multi-" |
| 40 | + "process access.", |
| 41 | + RuntimeWarning, |
| 42 | + stacklevel=2, |
| 43 | + ) |
| 44 | + |
| 45 | + |
| 46 | +@contextmanager |
| 47 | +def _locked_jsonl(path): |
| 48 | + """Open lessons.jsonl with an advisory exclusive flock held for the scope. |
| 49 | +
|
| 50 | + Creates the file if missing ('a+' mode, which also permits read). The |
| 51 | + lock is process-level on Unix via fcntl.flock — two appenders serialize, |
| 52 | + and a render() call wrapping its entire read-render-write cycle in this |
| 53 | + lock blocks concurrent appenders until the render is done. Windows falls |
| 54 | + through without locking (see module-level warning). |
| 55 | +
|
| 56 | + Note: within a single process, opening the same path twice yields two |
| 57 | + separate fds with separate flock states, so nesting `_locked_jsonl` |
| 58 | + around another `_locked_jsonl` in the same thread will deadlock. Call |
| 59 | + `_append_lesson_unlocked(fd, lesson)` instead when already inside a |
| 60 | + lock (e.g. migrate_legacy_bullets is deliberately called OUTSIDE the |
| 61 | + render lock to sidestep this). |
| 62 | + """ |
| 63 | + os.makedirs(os.path.dirname(path) or ".", exist_ok=True) |
| 64 | + f = open(path, "a+") |
| 65 | + try: |
| 66 | + if _HAS_FLOCK: |
| 67 | + fcntl.flock(f.fileno(), fcntl.LOCK_EX) |
| 68 | + yield f |
| 69 | + finally: |
| 70 | + if _HAS_FLOCK: |
| 71 | + try: |
| 72 | + fcntl.flock(f.fileno(), fcntl.LOCK_UN) |
| 73 | + except OSError: |
| 74 | + # Release-on-close is the kernel default; swallowing a late |
| 75 | + # release failure doesn't leak the lock. |
| 76 | + pass |
| 77 | + f.close() |
| 78 | + |
| 79 | + |
| 80 | +def _append_lesson_unlocked(f, lesson): |
| 81 | + """Write a lesson row to an already-open, already-locked jsonl file. |
| 82 | +
|
| 83 | + Use this only when you already hold the lock (via `_locked_jsonl`). |
| 84 | + Seeks to end first because 'a+' mode tracks position across reads |
| 85 | + and the caller may have read from the head. |
| 86 | + """ |
| 87 | + f.seek(0, os.SEEK_END) |
| 88 | + f.write(json.dumps(lesson) + "\n") |
| 89 | + f.flush() |
| 90 | + |
| 91 | + |
24 | 92 | def append_lesson(lesson, semantic_dir): |
25 | 93 | """Append a lesson to semantic/lessons.jsonl. Returns the written path.""" |
26 | 94 | os.makedirs(semantic_dir, exist_ok=True) |
27 | 95 | path = os.path.join(semantic_dir, LESSONS_JSONL) |
28 | | - with open(path, "a") as f: |
29 | | - f.write(json.dumps(lesson) + "\n") |
| 96 | + with _locked_jsonl(path) as f: |
| 97 | + _append_lesson_unlocked(f, lesson) |
30 | 98 | return path |
31 | 99 |
|
32 | 100 |
|
@@ -182,33 +250,56 @@ def render_lessons(semantic_dir): |
182 | 250 | lessons.jsonl before rendering, so upgrades from the old markdown-only |
183 | 251 | format don't silently erase past promotions. Deduplicates entries by |
184 | 252 | lesson id so a provisional-then-accepted lesson renders once, not twice. |
| 253 | +
|
| 254 | + Concurrency-safe: the entire read-render-write cycle runs under an |
| 255 | + exclusive flock on lessons.jsonl. A concurrent append_lesson() either |
| 256 | + lands BEFORE our load (we include it) or AFTER our write (it blocks |
| 257 | + on the flock, then will re-render on its own — graduate.py calls |
| 258 | + render_lessons right after appending). LESSONS.md is rewritten |
| 259 | + atomically via temp file + rename so readers never see a half-written |
| 260 | + file. |
185 | 261 | """ |
186 | | - # First, absorb any un-registered bullets below the sentinel |
| 262 | + # Migrate BEFORE taking the render lock. migrate_legacy_bullets calls |
| 263 | + # append_lesson internally, which acquires its own lock; nesting would |
| 264 | + # deadlock (two fds on the same file within one process each want |
| 265 | + # LOCK_EX). Migration is idempotent and only does real work on first |
| 266 | + # run after an upgrade, so the ordering is safe. |
187 | 267 | migrate_legacy_bullets(semantic_dir) |
188 | | - lessons = _dedupe_by_id(load_lessons(semantic_dir)) |
189 | | - auto_section = _build_auto_section(lessons) |
190 | 268 |
|
191 | | - path = os.path.join(semantic_dir, LESSONS_MD) |
192 | | - |
193 | | - if os.path.exists(path): |
194 | | - existing = open(path).read() |
195 | | - if SENTINEL in existing: |
196 | | - prefix = existing.split(SENTINEL)[0].rstrip() |
197 | | - new = f"{prefix}\n\n{SENTINEL}\n\n{auto_section}" |
198 | | - else: |
199 | | - new = existing.rstrip() + f"\n\n{SENTINEL}\n\n{auto_section}" |
200 | | - else: |
201 | | - header = ( |
202 | | - "# Lessons\n\n" |
203 | | - "> _Auto-managed below. Hand-curated preamble + seed lessons " |
204 | | - "above the sentinel are preserved across renders._\n" |
205 | | - ) |
206 | | - new = f"{header}\n{SENTINEL}\n\n{auto_section}" |
| 269 | + jsonl_path = os.path.join(semantic_dir, LESSONS_JSONL) |
| 270 | + md_path = os.path.join(semantic_dir, LESSONS_MD) |
207 | 271 |
|
208 | 272 | os.makedirs(semantic_dir, exist_ok=True) |
209 | | - with open(path, "w") as f: |
210 | | - f.write(new) |
211 | | - return path |
| 273 | + |
| 274 | + with _locked_jsonl(jsonl_path): |
| 275 | + lessons = _dedupe_by_id(load_lessons(semantic_dir)) |
| 276 | + auto_section = _build_auto_section(lessons) |
| 277 | + |
| 278 | + if os.path.exists(md_path): |
| 279 | + existing = open(md_path).read() |
| 280 | + if SENTINEL in existing: |
| 281 | + prefix = existing.split(SENTINEL)[0].rstrip() |
| 282 | + new = f"{prefix}\n\n{SENTINEL}\n\n{auto_section}" |
| 283 | + else: |
| 284 | + new = existing.rstrip() + f"\n\n{SENTINEL}\n\n{auto_section}" |
| 285 | + else: |
| 286 | + header = ( |
| 287 | + "# Lessons\n\n" |
| 288 | + "> _Auto-managed below. Hand-curated preamble + seed lessons " |
| 289 | + "above the sentinel are preserved across renders._\n" |
| 290 | + ) |
| 291 | + new = f"{header}\n{SENTINEL}\n\n{auto_section}" |
| 292 | + |
| 293 | + # Atomic rewrite: write to .tmp next to the target, then rename. |
| 294 | + # os.replace is atomic on POSIX and Windows (Python 3.3+), so a |
| 295 | + # reader of LESSONS.md always sees either the old or the new |
| 296 | + # complete content, never a half-written file. |
| 297 | + tmp_path = md_path + ".tmp" |
| 298 | + with open(tmp_path, "w") as f: |
| 299 | + f.write(new) |
| 300 | + os.replace(tmp_path, md_path) |
| 301 | + |
| 302 | + return md_path |
212 | 303 |
|
213 | 304 |
|
214 | 305 | def render_lessons_as_text(semantic_dir): |
|
0 commit comments