Skip to content

Synapse v2.1.12 — "hold less, drop nothing"

Patch release closing three defects found by reading the Marker conversion path, the watcher startup sequence and the graph cache during weekly maintenance. No schema migrations, no new endpoints, no API changes, no new configuration keys.

Like 2.1.8 through 2.1.11, none of these came from a live failure report. What they have in common is that each one is a guard that does not guard: a chunked read whose result is materialised anyway, an except that names the wrong exception, and an assert whose comment claims a branch is unreachable when it is not. In all three cases the surrounding code was written with the right intent — the defect is that the intent is not what the code does.

Fixed

A 300 MB PDF upload was buffered whole, twice, on a path built to stream it

POST /ingest/convert-marker reads each uploaded PDF in 65 KB chunks into a temp file, and it does so on purpose — the endpoint accepts files far larger than anything else in Synapse:

max_bytes: int = settings.marker_max_upload_bytes   # 314_572_800 — 300 MB
...
with open(tmp_fd, "wb") as tmp_file:
    chunk_size = 65_536
    while True:
        chunk = await upload.read(chunk_size)
        ...
        tmp_file.write(chunk)

The cap is 300 MB rather than the 25 MB generic max_upload_bytes by design (ADR-0065): Marker chunks large PDFs by page range, so this is the one endpoint where a multi-hundred-megabyte scanned book is an ordinary input, not an attack.

One line after that loop finished, the whole thing came back:

pdf_bytes = Path(tmp_name).read_bytes()      # ← up to 300 MB, resident
Path(tmp_name).unlink(missing_ok=True)
pdf_dst.write_bytes(pdf_bytes)

Root cause. A streamed read whose result is materialised anyway bounds nothing. The chunk loop did not reduce the peak; it only moved when the peak happens — from during the socket read to immediately after it, still inside the request handler, still per file, for up to ten files in one submission. The purpose of the loop was defeated by the copy that followed it.

The background driver then repeated the same shape on the way out to Marker:

pdf_bytes = Path(entry.pdf_abs_path).read_bytes()
...
files={"file": (entry.safe_stem + ".pdf", pdf_bytes, "application/pdf")}

The fix. The upload now lands with os.replace(). mkstemp() already created the temp file inside raw_sources (dir=str(raw_sources)), so the rename is same-filesystem and atomic by construction — no bytes pass through the process at all. The driver hands httpx an open file handle instead of a bytes object; httpx's multipart encoder reads a file object in 64 KB chunks (FileField.CHUNK_SIZE in httpx/_multipart.py) but yields a bytes value in one piece, so this is the difference between streaming and buffering at that layer.

Two smaller problems closed with it:

  • A truncated PDF was reachable. read_bytes() + write_bytes() is not atomic. A crash, OOM kill, or container stop between the two left a partial .pdf at the destination path — and both the watcher and the Marker driver would have treated it as a complete source. The rename cannot produce that state.
  • A leaked file descriptor was reachable. The driver's handle is closed in a finally, not after the POST. asyncio.CancelledError is a BaseException and passes straight through the except httpx.TimeoutException / except Exception chain around the request — the same exit that leaked the capability slot in 2.1.11. A batch of ten cancelled mid-run would otherwise have leaked ten descriptors for the life of the process.

One deliberate detail. mkstemp() creates its file 0600, while write_bytes() created 0644 under the usual umask. A bare rename would therefore have silently changed the permissions of every converted upload. raw/ sits next to a vault shared with Obsidian and LiveSync (I5/K7), so the temp file is chmod'd to 0644 before the move and the vault sees exactly what it saw before.

A log line could stop the whole backend from booting

VaultWatcher._emit_startup_notice() has one job: emit a single INFO line at startup if raw/sources/ is non-empty, telling the operator that pre-existing files are not auto-indexed (I1, ADR-0006). It writes nothing, touches no DB, triggers no ingest.

try:
    with os.scandir(watch_dir) as it:
        next(it)
        logger.info("startup: watching %s; pre-existing files are NOT auto-indexed (I1). ...")
except StopIteration:
    pass

Root cause. os.scandir() raises OSError, not just StopIteration. StopIteration is only the empty-directory case — the expected one. Nothing above caught the rest, so an OSError here propagated out of _emit_startup_notice(), out of VaultWatcher.start(), out of start_watcher(), and through the FastAPI lifespan: the service refused to start because it could not decide whether to log something.

No adversary is needed to reach it:

  • a vault volume the container can traverse but not read (+x, no +r) → PermissionError
  • a volume unmounted, or a bind-mount host path removed, between the mkdir(exist_ok=True) on the line above and this call → FileNotFoundError / NotADirectoryError
  • an I/O error on the underlying device → OSError

This is the same family as the 2.1.9 scheduler defect, where run_one_scan() wrapped its scandir in try/except OSError but not the mkdir() above it. Here the halves are reversed: the mkdir is covered by exist_ok, and the scandir is the unguarded one.

The fix. except OSError logs a WARNING naming the directory and the error, skips the notice, and lets startup continue. observer.schedule() a few lines below is left exactly as it was — that call really does decide whether a watch is possible, and it should still fail loudly. The informational line no longer has a vote.

GET /graph could answer 500 on the first request after boot

GraphCache.get_graph() ended on this line:

assert self._snapshot is not None  # mypy
return self._snapshot, False

Root cause. The comment asserts the branch exists only to satisfy the type checker. It does not. Walk the first GET /graph after a restart, when _snapshot is None and _marker is unset:

  1. Request A misses, finds no snapshot to serve stale, takes the inline branch, sets _in_flight = True, and awaits the FA2 recompute.
  2. Request B arrives while A is still computing. It misses, sees _in_flight, and calls _wait_for_in_flight() — which polls in 50 ms steps and gives up after 10 seconds:
async def _wait_for_in_flight(self) -> None:
    waited = 0.0
    while self._in_flight and waited < 10.0:
        await asyncio.sleep(0.05)
        waited += 0.05
  1. An FA2 layout on a large vault outlasts 10s routinely. When it does, B resumes with _in_flight still True and _snapshot still None. It skips the stale-while-revalidate branch (there is nothing to serve), skips the inline-recompute branch (if not self._in_flight), and falls onto the assert.

The result is an AssertionError escaping a plain read endpoint as a 500 — and under python -O, where asserts are stripped, something worse: a None snapshot returned to a caller that will fail somewhere less obvious. Two browser tabs, a React StrictMode double-effect, or a client retry are each enough to produce request B.

The fix. That branch now returns an empty GraphSnapshot(data_version=current_version) for the one request that reached it, which is exactly how the inline-recompute failure path a few lines above already degrades:

except Exception:
    logger.exception("GraphCache.get_graph: inline recompute failed")
    if self._snapshot is None:
        self._snapshot = GraphSnapshot(data_version=current_version)

_marker is deliberately not stamped, so the next GET /graph is still a MISS and picks up the real snapshot the moment the in-flight run lands. No second concurrent FA2 is started, so I2 is untouched — the layout still runs exactly once, server-side.

Also in this release

An entry was added to docs/reference/ROADMAP-3.0-IDEAS.md for a fourth finding that is not fixed here: AuthFailureRateLimitMiddleware calls its limiter after call_next() and only on a 401, so past the threshold the only effect is that the 401 is relabelled 429 — the attempt is still fully evaluated, and a correctly guessed token answers 200 and never reaches the limiter at all. Making it a real lockout is a behaviour change on an authentication path that can lock an operator out of every endpoint, so it needs a decided perimeter and escape hatch rather than an unattended weekly patch.

Upgrade notes

Nothing to do. Pull the image (or rebuild) and restart:

docker compose pull && docker compose up -d
  • No schema migration. No SQLAlchemy model changed, so docs/er/schema.mmd is unchanged and no Alembic revision ships with this release.
  • No API change. docs/api/openapi.json differs only in the version string. Request and response shapes for /ingest/convert-marker, /ingest/convert-marker/status and /graph are identical.
  • No new configuration. MARKER_MAX_UPLOAD_BYTES (300 MB) and INGEST_MAX_CONCURRENCY keep their meanings and defaults; nothing new to set in .env.
  • One observable difference, by design: a vault directory that cannot be read at startup no longer prevents the backend from starting. It logs startup: could not check <dir> for pre-existing files (...) at WARNING instead. If that line appears in your logs after upgrading, the watcher is very likely not working either — check the mount's permissions. Previously this condition presented as a container that would not boot at all, which was louder but strictly less useful.
  • Marker uploads keep 0644 permissions, as before. If you had scripts relying on converted PDFs being group/other-readable in raw/sources/, they are unaffected.