Hacktoberfest 2026: los issues que los mantenedores marcaron para octubre, abiertos y aptos para principiantes. Explorar issues de Hacktoberfest

EPU Parser: potential race condition

Abierto
  • ¿Algo no está bien?
#89 0 comentarios 0 reacciones 1 asignado Ver en GitHub

@vredchenko ya está trabajando en esto.

Desde el 14/2/2025.

Evaluación

Este issue todavía no se ha evaluado.

Descripción

Status

The startup race window is confirmed to exist in current main. Actual data loss is
plausible but unproven — it has never been reproduced. The original description of this
issue overstated both the certainty and the size of the failure, which is likely why attempts
to reproduce it have come up empty.

This issue is now scoped as: build a test that can catch it, then fix what the test proves.

Corrected mechanics

smartem-agent watch parses the existing tree to completion, and only then starts the observer
(src/smartem_agent/__main__.py):

logging.info("Parsing existing directory contents...")          # ~273
watcher.datastore = EpuParser.parse_epu_output_dir(watcher.datastore)
logging.info("..done! Now listening for new filesystem events")

observer = Observer()
observer.schedule(watcher, str(path), recursive=True)           # ~278
...
observer.start()                                                # ~296

Two corrections to the original write-up:

  1. The window is larger than previously stated. It is not "parse-end to schedule()".
    EpuParser.parse_epu_output_dir opens with
    list(datastore.root_dir.glob("**/*EpuSession.dm")) — a full recursive walk materialised
    at t0 — and the OS-level watch is not established until observer.start(). The exposure is
    the entire parse duration.

  2. The window is far narrower in effect than "any data arriving is dropped".
    SmartEMWatcherV2.watched_event_types is ["created", "modified"] (fs_watcher.py:39),
    not ["created"]. Any file still being written, or ever rewritten, when the observer starts
    is still picked up via its modified event. The only genuinely lost file is one that is
    created, finalised, and never touched again, entirely inside the window.

There is no mitigation elsewhere: on_any_event is the only event method in fs_watcher.py,
and there is no rescan, reconciliation, or periodic full-scan anywhere in the watcher. The
orphan manager handles out-of-order events, not missing ones.

Working hypothesis: only reachable on mid-session restart

On a fresh acquisition the initial glob finds almost nothing, so the window is near-zero and
the bug is probably not reachable at all. It should only open meaningfully when the agent
is restarted against a large, in-progress session, where the parse can run for a long time
while EPU keeps writing.

If correct, this explains the issue's history: anyone reproducing it the obvious way (start the
agent, drop files in) would see nothing and reasonably conclude it was not real.

Unknown, and not answerable from this repository

Whether EPU ever writes a matched file write-once-never-touch is the deciding fact, and it
is ThermoFisher's proprietary behaviour. If every EPU file gets a later modified event, this
bug does not exist in practice. That question is what the test needs to settle empirically.

Plan

  1. Catch it first. Add a regression test that reproduces the window deterministically
    before changing any production code. Shape: pre-populate a large EPU tree, begin
    watch, write additional files during the parse, then assert the resulting datastore is
    equivalent to a clean full parse of the final tree. EPUPlayer
    (smartem-devtools/packages/smartem-epuplayer/) is the natural driver for the concurrent
    writes. Home: tests/smartem_agent/test_fs_watcher.py.
    To make the window wide enough to hit reliably, the parse should be slowed via injection
    (a hook or monkeypatched parse_epu_output_dir) rather than by building a genuinely huge
    fixture.
  2. Fix it. Only once the test fails for the right reason. Likely direction: start the
    observer before the parse and buffer events until the parse completes, reconciling
    against the parsed datastore (writes are already idempotent by UUID). Alternatives
    considered: post-start revalidation scan; atomic transition.
  3. Retain the test. It stays in the suite as a permanent regression guard.

Run it on Windows as well as Linux

Production agents run on Windows EPU workstations, and the two platforms fail differently
here — in both directions. A green Linux suite is not evidence about production.

  • Linux (inotify): inotify is not recursive. Watchdog registers a watch per subdirectory by
    walking the tree at emitter start, and only adds watches for new subdirectories when it
    processes their creation event. EPU creates deep subdirectories constantly
    (GridSquare_*/FoilHoles/...), so there is a second, independent race: files landing in a
    brand-new subdirectory before its watch is registered. Linux-only.
  • Windows (ReadDirectoryChangesW): a single handle with bWatchSubtree=TRUE covers the
    whole subtree, so the per-subdirectory race does not arise. However the kernel notification
    buffer can overflow under burst load and drop events wholesale — a documented characteristic
    of that API that watchdog surfaces poorly. Windows-only, and closer to real acquisition load.

The test should assert the same invariant on both platforms.

This is a small CI change, not a new pipeline:

  • .github/workflows/ci.yml:44 currently reads
    runs-on: ["ubuntu-latest"] # can add windows-latest, macos-latest.
  • _test.yml already takes runs-on as an input, so the matrix is parameterised.
  • build_win_smartem_agent.yml already runs on windows-latest, so the Windows toolchain is
    proven to install.

Note that adding windows-latest to the full test matrix will also surface any unrelated
Windows failures in the existing suite; consider a dedicated job for the filesystem tests if
that turns out to be noisy.

Code references

  • src/smartem_agent/__main__.py — watch command, ~273-296
  • src/smartem_agent/fs_watcher.py:39 — watched_event_types
  • src/smartem_agent/fs_watcher.py:433 — on_any_event (sole event method)
  • src/smartem_agent/fs_parser.py:963 — parse_epu_output_dir
Lenguaje dominante
TypeScript
Estrellas
0
Forks
0
Sin PR fusionados en 30 d

Preparar el entorno

Este proyecto no incluye contenedor de desarrollo, Dockerfile ni guía de contribución, así que la configuración corre por tu cuenta: empieza por su README y consulta nuestra guía para la primera contribución para los pasos generales.

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de DiamondLightSource/smartem-devtools

Todos los issues de DiamondLightSource/smartem-devtools

Issues similares

Más issues de TypeScript

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.