singleton: guard the state, not the program's name
El SDK CI - dev / build-and-test (pull_request) Failing after 4m6s
El SDK CI - dev / build-and-test (pull_request) Failing after 4m6s
The singleton lock protected a filename, not a store. It was keyed on $EL_SINGLETON_DIR|$TMPDIR|/tmp + /el-singleton-<program>.lock — the program's NAME and a temp directory — and never consulted the state it claimed to protect, while its own refusal message read "Refusing to start a second instance against the same state." Measured, it failed in both directions. A second engram against a DIFFERENT data dir was refused, naming the first's pid. And TMPDIR=/tmp/other let a second engram start against the SAME data dir with no complaint — the two-writer data-loss condition the guard exists to prevent, defeated by one environment variable. Both are one error: the identity of the resource had been replaced by a label for it. The lock now lives inside the state it guards — <state>/.el-singleton-<id>.lock — and the program block says what that state is. Same directory is the same file is the same inode, so it contends and there is no TMPDIR left in the key to change. Different directories are different files, so they don't. Different spellings of one directory (trailing slash, x/../x, symlink) collapse in the kernel's own path walk, so they contend without this code comparing strings; canonicalisation is for the message, never the decision. `guards:` is an expression so a program can point at the resolver that already owns its path — guards: engram_resolve_data_dir() — instead of restating that resolver's default, which is the two-owners defect spec 18.4 exists to prevent. A `singleton:` without `guards:` is now a compile error; emitting a name-keyed lock instead would be emitting the defect. Kept: the flock (the kernel drops it on crash and SIGKILL, so there is still no "delete the lock file to get unstuck" ritual — a stale file inside a copied data dir is inert), and the holder's pid in the message. Changed: the message is true. It says "the same state" because the lock it failed to take is in that state, and it names the state it checked. An unguardable state (missing, read-only) now refuses rather than starting unguarded. Also corrects lang/AGENTS.md's compiler rebuild line, which had gone stale: linking el_runtime.c alone no longer resolves.
This commit is contained in:
+96
-19
@@ -19809,22 +19809,84 @@ void log_warn(el_val_t msg_v) {
|
||||
* become a convention. */
|
||||
static int el_singleton_fd = -1;
|
||||
static char el_singleton_path[1024];
|
||||
static char el_singleton_state[1024];
|
||||
|
||||
static const char* el_singleton_dir(void) {
|
||||
const char* d = getenv("EL_SINGLETON_DIR");
|
||||
if (d && *d) return d;
|
||||
d = getenv("TMPDIR");
|
||||
if (d && *d) return d;
|
||||
return "/tmp";
|
||||
}
|
||||
|
||||
/* el_singleton_acquire — claim exclusive process identity, or refuse to start.
|
||||
* Compiler-injected as the FIRST statement of main() for any program whose
|
||||
* `program` block declares `singleton:`. */
|
||||
el_val_t el_singleton_acquire(el_val_t id_v) {
|
||||
/* el_singleton_acquire — claim exclusive use of the guarded STATE, or refuse to
|
||||
* start. Compiler-injected as the FIRST statement of main() for any program
|
||||
* whose `program` block declares `singleton:` (which must also declare
|
||||
* `guards:` — see lang/spec/language.md §18.2).
|
||||
*
|
||||
* GUARD THE THING, NOT THE NAME.
|
||||
*
|
||||
* Until 2026-08-16 this lock was keyed on the program's NAME and on $TMPDIR —
|
||||
* `$EL_SINGLETON_DIR|$TMPDIR|/tmp` + `/el-singleton-<name>.lock` — and never
|
||||
* consulted the state it claimed to protect. Its own refusal message said
|
||||
* "Refusing to start a second instance against the same state" while it had not
|
||||
* looked at any state. Measured, it failed in BOTH directions:
|
||||
*
|
||||
* - FALSE POSITIVE: two engrams against genuinely DIFFERENT data dirs could
|
||||
* not coexist. The second was refused, naming the first's pid — for sharing
|
||||
* a name, not a store.
|
||||
* - FALSE NEGATIVE (the dangerous one): `TMPDIR=/tmp/other` let a second
|
||||
* instance start against the SAME data dir with no complaint. That is
|
||||
* exactly the two-instance data-loss condition the guard exists to prevent,
|
||||
* and the workaround was one environment variable.
|
||||
*
|
||||
* Both are one error: the identity of the resource had been replaced by a label
|
||||
* for it. The fix is to put the lock file INSIDE the state it guards:
|
||||
*
|
||||
* <state>/.el-singleton-<id>.lock
|
||||
*
|
||||
* That placement is the whole mechanism, and it is why there is no hashing, no
|
||||
* canonical-path registry, and no environment variable left to subvert:
|
||||
*
|
||||
* - Same directory => same file => same inode => the flock CONTENDS. There is
|
||||
* no TMPDIR in the key, so there is nothing to change to get past it.
|
||||
* - Different dirs => different files => no contention. Two stores are two
|
||||
* stores; they were never in conflict and are no longer treated as if they
|
||||
* were.
|
||||
* - Different SPELLINGS of one directory — trailing slash, `x/../x`, a symlink
|
||||
* — resolve to the same inode in the kernel's own path walk, so they contend
|
||||
* without this code comparing strings at all. Path canonicalisation here is
|
||||
* for the human-readable message, never for the decision.
|
||||
*
|
||||
* Kept, deliberately, from the version this replaces: it is an flock and not a
|
||||
* pidfile (the kernel releases it on crash and on SIGKILL, so there is no stale
|
||||
* state and therefore no "delete the lock file to get unstuck" ritual), and it
|
||||
* reports the HOLDER'S PID (added because a stale process survived `pkill -f`
|
||||
* and went on answering probes; "already running" is not actionable, a pid is).
|
||||
*
|
||||
* Changed: the message is now TRUE. It says "the same state" because the lock it
|
||||
* failed to take lives in that state, and it names the state it checked. */
|
||||
el_val_t el_singleton_acquire(el_val_t id_v, el_val_t state_v) {
|
||||
const char* id = EL_CSTR(id_v);
|
||||
if (!id || !*id) return EL_NULL;
|
||||
|
||||
/* A singleton with nothing to guard is the defect this function exists to
|
||||
* remove; refuse rather than silently fall back to name-keying. The compiler
|
||||
* rejects `singleton:` without `guards:`, so reaching this is a toolchain
|
||||
* mismatch, not a user mistake — say so. */
|
||||
const char* state = EL_CSTR(state_v);
|
||||
if (!state || !*state) {
|
||||
fprintf(stderr,
|
||||
"[el] FATAL: singleton '%s' was given no state to guard.\n"
|
||||
"[el] A lock keyed on a program's NAME instead of on the state it\n"
|
||||
"[el] protects is not a guard: it refuses unrelated instances and\n"
|
||||
"[el] permits concurrent ones. Declare `guards: <path>` alongside\n"
|
||||
"[el] `singleton:` in the program block (spec §18.2).\n", id);
|
||||
exit(1);
|
||||
}
|
||||
|
||||
/* Canonicalise so the operator is told WHICH directory was checked, in one
|
||||
* spelling, whatever spelling they typed. This is a readability measure, not
|
||||
* the mechanism: realpath() may fail (the directory may not exist yet) and
|
||||
* correctness must not depend on it — when it succeeds it names the same
|
||||
* directory, and when it does not we fall back to the path as given and the
|
||||
* kernel's own path walk still collapses the spellings at open() time. */
|
||||
char* rp = realpath(state, NULL);
|
||||
snprintf(el_singleton_state, sizeof(el_singleton_state), "%s", rp ? rp : state);
|
||||
free(rp);
|
||||
|
||||
/* Sanitise the id into a filename. */
|
||||
char safe[256];
|
||||
size_t si = 0;
|
||||
@@ -19835,13 +19897,25 @@ el_val_t el_singleton_acquire(el_val_t id_v) {
|
||||
safe[si++] = (char)(ok ? c : '-');
|
||||
}
|
||||
safe[si] = '\0';
|
||||
/* THE MECHANISM: the lock lives inside the state it guards. Two spellings of
|
||||
* one directory name one file; two directories name two files. Note there is
|
||||
* no $TMPDIR and no $EL_SINGLETON_DIR in this path — the escape hatch that
|
||||
* made the guard bypassable is gone because there is nowhere left to put it. */
|
||||
snprintf(el_singleton_path, sizeof(el_singleton_path),
|
||||
"%s/el-singleton-%s.lock", el_singleton_dir(), safe);
|
||||
"%s/.el-singleton-%s.lock", el_singleton_state, safe);
|
||||
|
||||
int fd = open(el_singleton_path, O_RDWR | O_CREAT, 0644);
|
||||
if (fd < 0) {
|
||||
fprintf(stderr, "[el] FATAL: singleton '%s': cannot open lock file %s: %s\n",
|
||||
id, el_singleton_path, strerror(errno));
|
||||
/* Unguardable state. Refusing is the only honest option: starting anyway
|
||||
* would mean running unguarded against exactly the store the guard is
|
||||
* here to protect. */
|
||||
fprintf(stderr,
|
||||
"[el] FATAL: singleton '%s': cannot open the lock inside the state it guards.\n"
|
||||
"[el] state: %s\n"
|
||||
"[el] lock: %s (%s)\n"
|
||||
"[el] The guarded directory must exist and be writable. Refusing to\n"
|
||||
"[el] start unguarded against it.\n",
|
||||
id, el_singleton_state, el_singleton_path, strerror(errno));
|
||||
exit(1);
|
||||
}
|
||||
if (flock(fd, LOCK_EX | LOCK_NB) != 0) {
|
||||
@@ -19857,11 +19931,14 @@ el_val_t el_singleton_acquire(el_val_t id_v) {
|
||||
fprintf(stderr, "[el] FATAL: another instance of '%s' is already running", id);
|
||||
if (holder > 0) fprintf(stderr, " (pid %ld)", holder);
|
||||
fprintf(stderr, ".\n"
|
||||
"[el] lock: %s\n"
|
||||
"[el] state: %s\n"
|
||||
"[el] lock: %s\n"
|
||||
"[el] Refusing to start a second instance against the same\n"
|
||||
"[el] state. Stop the running one and VERIFY it is gone\n"
|
||||
"[el] (ps -p <pid>) before retrying.\n",
|
||||
el_singleton_path);
|
||||
"[el] state. Two writers against one store is data loss, not a\n"
|
||||
"[el] warning. Stop the running one and VERIFY it is gone\n"
|
||||
"[el] (ps -p %ld) before retrying — or point this instance at a\n"
|
||||
"[el] different state, which is permitted and is not refused.\n",
|
||||
el_singleton_state, el_singleton_path, holder > 0 ? holder : (long)0);
|
||||
close(fd);
|
||||
exit(1);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user