Skip to content

Commit ef8906a

Browse files
committed
Clear a stale .done marker before reusing a dump path
The completion marker is meant to tell a watcher the .rdump is fully written. On a fixed (reused) dump path the previous run's .done lingered while the new dump was being truncated and rewritten, so a watcher could ship the half-written file -- exactly what the marker should prevent. Remove the marker before truncation and re-create it only on completion. CI now pre-plants a non-empty .done and requires it to come back empty. https://claude.ai/code/session_017skw8BsHkF8wur7Pf4G9W5
1 parent 7b3ff55 commit ef8906a

3 files changed

Lines changed: 36 additions & 6 deletions

File tree

README.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,9 @@ rdump.oom_dump_marker=1 ; after a complete dump, also create "<path>.done"
167167

168168
The `<path>.done` file is created (empty, `0600`) only once the dump is fully
169169
written and closed — wait for `oom-1234.rdump.done`, then ship
170-
`oom-1234.rdump`. This gives the atomic-visibility guarantee a temp-file +
170+
`oom-1234.rdump`. When a dump reuses a path, the old marker is removed *before*
171+
the new dump is truncated and re-created only when it completes, so a `.done` is
172+
never stale over a half-rewritten file. This gives the atomic-visibility guarantee a temp-file +
171173
`rename` would, without the rename's downsides, and works on the OOM death path
172174
(it's a plain `open`/`close`). There is deliberately **no PHP completion
173175
callback**: the OOM dump runs inside the engine's error handler on a process that

ci/build-and-test.sh

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -243,16 +243,23 @@ rm -rf "$BUDGET_DIR"
243243
# --- OOM dump completion marker -------------------------------------
244244
# With rdump.oom_dump_marker=1 a "<path>.done" must appear once the dump is
245245
# fully written, so a directory watcher can wait for it instead of racing the
246-
# half-written .rdump.
246+
# half-written .rdump. A stale marker from a previous dump to the same path
247+
# must be cleared first: pre-plant a non-empty .done and require it to come
248+
# back empty (cleared, then re-created on completion), never left stale.
247249
MARKER_DIR=/tmp/rdump-marker
248250
rm -rf "$MARKER_DIR"; mkdir -p "$MARKER_DIR"
251+
MARKER_DUMP="$MARKER_DIR/fixed.rdump"
252+
printf STALE > "$MARKER_DUMP.done"
249253
php -d extension="$SO" -d memory_limit=8M \
250-
-d rdump.oom_dump="$MARKER_DIR/oom-%p.rdump" -d rdump.oom_dump_marker=1 \
254+
-d rdump.oom_dump="$MARKER_DUMP" -d rdump.oom_dump_marker=1 \
251255
-r 'function r(){ r(); } r();' >/dev/null 2>&1 || true
252-
if ls "$MARKER_DIR"/oom-*.rdump.done >/dev/null 2>&1; then
253-
echo "oom-marker OK"
254-
else
256+
if [ ! -f "$MARKER_DUMP.done" ]; then
255257
echo "::error::rdump.oom_dump_marker did not write a .done marker"
256258
exit 1
257259
fi
260+
if [ -s "$MARKER_DUMP.done" ]; then
261+
echo "::error::a stale .done marker survived a dump to the same path"
262+
exit 1
263+
fi
264+
echo "oom-marker OK"
258265
rm -rf "$MARKER_DIR"

rdump.c

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -611,6 +611,20 @@ static int rdump_expand_path(const char *tmpl, char *buf, size_t buf_sz)
611611
return 0;
612612
}
613613

614+
/* Remove a stale "<dump_path>.done" before a dump is (re)written to the same
615+
* path. Without this, a fixed-path dump would keep last run's marker present
616+
* while the new dump is mid-write, telling a watcher the half-written file is
617+
* already complete. Cleared before truncation, re-created only on success. */
618+
static void rdump_clear_done_marker(const char *dump_path)
619+
{
620+
char marker[4096];
621+
if ((size_t)snprintf(marker, sizeof(marker), "%s.done", dump_path)
622+
>= sizeof(marker)) {
623+
return;
624+
}
625+
unlink(marker);
626+
}
627+
614628
/* Drop a "<dump_path>.done" marker once a dump is fully written and closed.
615629
* A directory watcher can wait for this instead of the .rdump, so it never
616630
* ships a half-written file -- the atomic-visibility benefit of a temp+rename
@@ -725,6 +739,13 @@ static void rdump_zend_error_cb(RDUMP_ERROR_CB_PARAMS)
725739
RDUMP_G(oom_dump_count)++;
726740
RDUMP_G(oom_dump_last_ts) = now;
727741

742+
/* Drop any prior completion marker before truncating the dump,
743+
* so a watcher never sees a stale .done over a half-rewritten
744+
* file when the same path is reused. */
745+
if (RDUMP_G(oom_dump_marker)) {
746+
rdump_clear_done_marker(out_path);
747+
}
748+
728749
char *err = NULL;
729750
RDUMP_G(in_oom_dump) = 1;
730751
/* Best-effort: this is the death path, so swallow any failure. */

0 commit comments

Comments
 (0)