diff --git a/CHANGELOG.md b/CHANGELOG.md index b145b06..442130c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ All notable changes to this project are recorded here. The format follows Keep a ### Fixed +- Semaphore capacity is validated as a bounded positive decimal and normalized before storage, arithmetic, and JSON serialization. Leading-zero values such as `01`, `08`, and `010` keep their decimal meaning, including when reading metadata written by older versions. Invalid stored capacities fail with `store-read` (#35). +- `doctor` reads a stored semaphore capacity with the same decimal rule. Before, a legacy `08` printed a bash arithmetic error, `010` was compared as octal eight (so nine live slots were a false `sem-capacity` finding), and a capacity past 2^64 wrapped around to a small number instead of being a `sem-record` finding. - Validate authoritative lock and semaphore records before normal reads or planning (#33). Corrupt records now produce a structured `store-read` error with exit 2 before any success output or mutation. Doctor shares the decoder and safely reports malformed numeric fields; generation tokens remain opaque. Stored decimal fields normalize leading zeros and reject values outside the nonnegative signed 64-bit range. Child admission refuses a parent whose family generation cannot advance without overflow. - `sem list` no longer reads the slot of a job named `meta` as a second semaphore. It matched any ref ending in `/meta`, so `refs/locks/sem/gpu/slots/meta` printed a `gpu/slots` line with an empty capacity, which is not valid JSON. Snapshot validation splits semaphore refs the same way, so such a slot is validated as a slot rather than as metadata. - Path normalisation preserves literal `*`, `?` and bracket characters instead of expanding them against files in the working tree. diff --git a/Makefile b/Makefile index 21a09dd..96a514e 100644 --- a/Makefile +++ b/Makefile @@ -14,12 +14,13 @@ lint: test: bash test/test.sh + python3 test/capacity.py bash test/literal-paths.sh bash test/unicode-locale.sh bash test/unicode-locale-calibration.sh test-docker: # the same suite inside the official bash image, for a wall between the tests and your machine - docker run --rm -v "$(CURDIR)":/src -w /src bash:5.2 bash -c 'apk add --no-cache git python3 py3-jsonschema >/dev/null && git config --global user.email t@example.invalid && git config --global user.name t && bash test/test.sh && bash test/literal-paths.sh && bash test/unicode-locale.sh && bash test/unicode-locale-calibration.sh' + docker run --rm -v "$(CURDIR)":/src -w /src bash:5.2 bash -c 'apk add --no-cache git python3 py3-jsonschema >/dev/null && git config --global user.email t@example.invalid && git config --global user.name t && bash test/test.sh && python3 test/capacity.py && bash test/literal-paths.sh && bash test/unicode-locale.sh && bash test/unicode-locale-calibration.sh' install: mkdir -p $(PREFIX)/bin diff --git a/README.md b/README.md index c208653..4fb6009 100644 --- a/README.md +++ b/README.md @@ -261,6 +261,8 @@ In summary, "all or nothing" is never a loop with a rollback. It is one stanza, A path lock says one holder. Some resources are better described by a number: two GPUs, five build agents. This section shows how git-locks gives a named resource a capacity while keeping the same atomicity, and it needs one new idea, a generation token, that the reader has all the pieces for. +Capacity is a positive decimal integer from `1` through `9223372036854775807`. Leading zeros are accepted and normalized: `--capacity 010` means ten and emits JSON `10`. Reads also normalize leading-zero capacities stored by older versions without rewriting their metadata. Invalid or out-of-range input is refused before semaphore refs are created; invalid stored capacity is a `store-read` error, and `doctor` reports it as a `sem-record` finding. + A semaphore is three kinds of ref under `refs/locks/sem//`. `meta` points at a blob holding the capacity. `slots/` is one ref per holder, pointing at a slot record with the holder and an expiry, exactly like a lock record. And `gen` points at a blob whose only purpose is to change: every transaction on the semaphore writes a fresh generation blob and `update`s `gen` from the generation it read. Two acquirers that both read "2 of 3 live" both try to move `gen` from the same old value; git lets exactly one through, and the other re-reads and finds the semaphore full. Here is the example's semaphore, capacity 2, filled by alice and bob, then refused to carol: ```text diff --git a/bin/git-locks b/bin/git-locks index 83127f4..326b26b 100755 --- a/bin/git-locks +++ b/bin/git-locks @@ -1908,6 +1908,10 @@ with_release_sem() { # record -> releases this invocation's slot, if it is still # acquirers who both counted "n of N live" contend on one compare-and-swap and # exactly one commits. The other re-reads. +valid_capacity() { # VAR value: record_uint (leading zeros are decimal, never octal; signed 64-bit range), and positive + record_uint "$1" "$2" && [[ "${!1}" != 0 ]] +} + sem_meta_ref() { printf '%s/sem/%s/meta' "${NS}" "$1"; } sem_gen_ref() { printf '%s/sem/%s/gen' "${NS}" "$1"; } sem_slot_ref() { printf '%s/sem/%s/slots/%s' "${NS}" "$1" "$2"; } @@ -1946,6 +1950,7 @@ sem_read() { # name -> 0, or 1 when the semaphore does not exist SEM_META_OID="$(ref_oid "${mref}")" [[ -n "${SEM_META_OID}" ]] || return 1 SEM_CAP="$(field "${SEM_META_OID}" capacity)" + valid_capacity SEM_CAP "${SEM_CAP}" || store_error "semaphore ${name} has an invalid capacity" gref="$(sem_gen_ref "${name}")" SEM_GEN_OID="$(ref_oid "${gref}")" SLOT_JOBS=() @@ -2183,7 +2188,7 @@ cmd_sem() { done case "${verb}" in create) - [[ "${capacity}" =~ ^[0-9]+$ && "${capacity}" -gt 0 ]] || fail '--capacity is a positive number' 2 + valid_capacity capacity "${capacity}" || fail '--capacity is a decimal integer from 1 through 9223372036854775807' 2 if sem_read "${name}"; then sem_refusal "${name}" exists exit 1 diff --git a/lib/170-semaphores.sh b/lib/170-semaphores.sh index ac9366b..9fd65fd 100644 --- a/lib/170-semaphores.sh +++ b/lib/170-semaphores.sh @@ -5,6 +5,10 @@ # acquirers who both counted "n of N live" contend on one compare-and-swap and # exactly one commits. The other re-reads. +valid_capacity() { # VAR value: record_uint (leading zeros are decimal, never octal; signed 64-bit range), and positive + record_uint "$1" "$2" && [[ "${!1}" != 0 ]] +} + sem_meta_ref() { printf '%s/sem/%s/meta' "${NS}" "$1"; } sem_gen_ref() { printf '%s/sem/%s/gen' "${NS}" "$1"; } sem_slot_ref() { printf '%s/sem/%s/slots/%s' "${NS}" "$1" "$2"; } @@ -43,6 +47,7 @@ sem_read() { # name -> 0, or 1 when the semaphore does not exist SEM_META_OID="$(ref_oid "${mref}")" [[ -n "${SEM_META_OID}" ]] || return 1 SEM_CAP="$(field "${SEM_META_OID}" capacity)" + valid_capacity SEM_CAP "${SEM_CAP}" || store_error "semaphore ${name} has an invalid capacity" gref="$(sem_gen_ref "${name}")" SEM_GEN_OID="$(ref_oid "${gref}")" SLOT_JOBS=() @@ -280,7 +285,7 @@ cmd_sem() { done case "${verb}" in create) - [[ "${capacity}" =~ ^[0-9]+$ && "${capacity}" -gt 0 ]] || fail '--capacity is a positive number' 2 + valid_capacity capacity "${capacity}" || fail '--capacity is a decimal integer from 1 through 9223372036854775807' 2 if sem_read "${name}"; then sem_refusal "${name}" exists exit 1 diff --git a/test/capacity.py b/test/capacity.py new file mode 100644 index 0000000..b383f16 --- /dev/null +++ b/test/capacity.py @@ -0,0 +1,127 @@ +#!/usr/bin/env python3 +"""Exercise decimal capacity through the CLI with an independent integer oracle.""" +import concurrent.futures +import json +import os +from pathlib import Path +import random +import subprocess +import tempfile + +import jsonschema + +ROOT = Path(__file__).resolve().parents[1] +CLI = ROOT / 'bin/git-locks' +SCHEMA = json.loads((ROOT / 'schema/git-locks.schema.json').read_text()) +VALIDATOR = jsonschema.Draft202012Validator(SCHEMA) +SEED = 3507 +LIMIT = 9223372036854775807 +checks = 0 + + +def check(condition, message): + global checks + assert condition, message + checks += 1 + + +with tempfile.TemporaryDirectory(prefix='git-locks-capacity-') as tmp: + base = Path(tmp) + env = {key: value for key, value in os.environ.items() + if not key.startswith('GIT_')} + env.update(HOME=tmp, GIT_LOCKS_STORE=str(base / 'store.git'), + GIT_LOCKS_NOW='1000000', LC_ALL='C') + + def run(*args, expected=0): + result = subprocess.run([str(CLI), *args], cwd=tmp, env=env, + text=True, capture_output=True, timeout=45) + check(result.returncode == expected, + f'{args}: exit {result.returncode}, expected {expected}: {result.stderr}') + data = [] + for output in (result.stdout, result.stderr): + for line in output.splitlines(): + value = json.loads(line) + VALIDATOR.validate(value) + data.append(value) + check(bool(data), f'{args}: missing structured output') + return data + + def git(*args, input=None): + return subprocess.run(['git', '--git-dir=' + env['GIT_LOCKS_STORE'], *args], + cwd=tmp, env=env, text=True, input=input, + capture_output=True, check=True).stdout.strip() + + rng = random.Random(SEED) + values = ['1', '01', '08', '010', str(LIMIT), '000' + str(LIMIT), '0' * 256 + '8'] + values += ['0' * rng.randrange(1, 25) + str(rng.randrange(1, 1000000)) for _ in range(32)] + for index, value in enumerate(values): + name = f'valid-{index}' + want = int(value, 10) + check(run('sem', 'create', name, '--capacity', value)[0]['capacity'] == want, + f'create did not normalize {value!r}') + check(run('sem', 'show', name)[0]['capacity'] == want, 'show capacity mismatch') + meta = git('show', f'refs/locks/sem/{name}/meta') + check(f'capacity: {want}' in meta.splitlines(), 'stored capacity is not canonical') + check(run('sem', 'acquire', name, '--job', 'a', '--holder', 'alice')[0]['capacity'] == want, + 'acquire capacity mismatch') + check(run('sem', 'release', name, '--job', 'a')[0]['capacity'] == want, + 'release capacity mismatch') + run('sem', 'delete', name) + + invalid = ['', '0', '00', '-1', '+1', '1.0', '1e2', ' 1', '1 ', '08x', '12', + str(LIMIT + 1), str(2**64 + 1), '9' * 100, '0' * 100 + str(LIMIT + 1)] + invalid += [str(rng.randrange(LIMIT + 1, 2**100)) for _ in range(16)] + before = git('for-each-ref', '--format=%(refname) %(objectname)', 'refs/locks/') + for index, value in enumerate(invalid): + result = run('sem', 'create', f'invalid-{index}', '--capacity', value, expected=2) + check(result[0]['reason'] == 'usage', 'invalid capacity is not a usage error') + check(git('for-each-ref', '--format=%(refname) %(objectname)', 'refs/locks/') == before, + 'invalid input changed authoritative refs') + + # Stores written by 0.7.0 may have leading zeros; reads normalize without rewriting. + run('sem', 'create', 'legacy', '--capacity', '1') + meta = git('show', 'refs/locks/sem/legacy/meta') + oid = git('hash-object', '-w', '--stdin', input=meta.replace('capacity: 1', 'capacity: 01') + '\n') + git('update-ref', 'refs/locks/sem/legacy/meta', oid) + check(run('sem', 'show', 'legacy')[0]['capacity'] == 1, 'legacy show is not normalized') + check(run('sem', 'list')[0]['capacity'] == 1, 'legacy list is not normalized') + check(git('rev-parse', 'refs/locks/sem/legacy/meta') == oid, 'read rewrote legacy metadata') + run('sem', 'acquire', 'legacy', '--job', 'first', '--holder', 'alice') + refused = run('sem', 'acquire', 'legacy', '--job', 'second', '--holder', 'bob', expected=1)[0] + check(refused['reason'] == 'capacity' and refused['capacity'] == 1, 'legacy capacity not enforced') + run('sem', 'release', 'legacy', '--job', 'first') + run('sem', 'delete', 'legacy') + + # A fresh metadata record can be corrupt independently of its JSON serialization. + for value in ('0', '08x', str(LIMIT + 1)): + run('sem', 'create', 'bad-meta', '--capacity', '1') + meta = git('show', 'refs/locks/sem/bad-meta/meta') + original = git('rev-parse', 'refs/locks/sem/bad-meta/meta') + oid = git('hash-object', '-w', '--stdin', input=meta.replace('capacity: 1', 'capacity: ' + value) + '\n') + git('update-ref', 'refs/locks/sem/bad-meta/meta', oid) + result = run('sem', 'show', 'bad-meta', expected=2) + check(result[0]['reason'] == 'store-read', 'bad stored capacity did not fail closed') + check(git('rev-parse', 'refs/locks/sem/bad-meta/meta') == oid, 'bad read changed metadata') + git('update-ref', 'refs/locks/sem/bad-meta/meta', original) + run('sem', 'delete', 'bad-meta') + + run('sem', 'create', 'race', '--capacity', '03') + + def contender(index): + result = subprocess.run([str(CLI), 'sem', 'acquire', 'race', '--job', f'r{index}', + '--holder', f'h{index}'], cwd=tmp, env=env, text=True, + capture_output=True, timeout=45) + assert result.returncode in (0, 1), result + for line in (result.stdout + result.stderr).splitlines(): + VALIDATOR.validate(json.loads(line)) + return result.returncode + + with concurrent.futures.ThreadPoolExecutor(max_workers=12) as pool: + statuses = list(pool.map(contender, range(12))) + check(statuses.count(0) == 3, f'12 racers on capacity 03 had {statuses.count(0)} winners') + observed = run('sem', 'show', 'race')[0] + check(observed['capacity'] == 3 and observed['live'] == 3 and len(observed['slots']) == 3, + 'post-race slot state violates capacity') + check(run('doctor')[0]['healthy'], 'post-race doctor unhealthy') + +print(f'capacity: {checks} checks passed; seed {SEED}; {len(values)} valid and {len(invalid)} invalid inputs; 12 racers / 3 winners') diff --git a/test/test.sh b/test/test.sh index 5475c79..11cda61 100755 --- a/test/test.sh +++ b/test/test.sh @@ -727,6 +727,20 @@ valid "sem delete line" "${out}" git-locks sem show batch >/dev/null 2>&1 check "a deleted semaphore is gone" "$?" "1" +# Decimal capacity must preserve its numeric value in storage and every JSON line. +for capacity_case in '01:1' '08:8' '010:10'; do + capacity_input="${capacity_case%:*}" + capacity_want="${capacity_case#*:}" + out="$(git-locks sem create "decimal-${capacity_want}" --capacity "${capacity_input}" 2>&1)" + check "capacity ${capacity_input} creates a semaphore" "$?" "0" + jfields "capacity ${capacity_input} emits decimal ${capacity_want}" "${out}" "capacity=${capacity_want}" + valid "capacity ${capacity_input} create line" "${out}" + out="$(git-locks sem show "decimal-${capacity_want}" 2>&1)" + check "capacity ${capacity_input} can be read" "$?" "0" + jfields "capacity ${capacity_input} reads decimal ${capacity_want}" "${out}" "capacity=${capacity_want}" + valid "capacity ${capacity_input} show line" "${out}" +done + # exactly K winners under contention git-locks sem create race --capacity 3 >/dev/null 2>&1 wins=0 @@ -1233,6 +1247,33 @@ findings n "${out}" sem-gen check "a semaphore without its gen ref is one sem-gen finding" "${n}" "1" valid "semaphore finding lines" "${out}" +# Doctor reads a stored capacity with the same decimal rule as sem_read: leading zeros are decimal, out of range is a sem-record finding. +git-locks sem create legacy --capacity 1 >/dev/null 2>&1 +legacy_meta="$(gs rev-parse refs/locks/sem/legacy/meta)" +for capacity_case in '08:0' '010:0' '9223372036854775808:1' '18446744073709551617:1'; do + capacity_stored="${capacity_case%:*}" + capacity_findings="${capacity_case#*:}" + rewritten="$(gs cat-file -p "${legacy_meta}" | sed "s/^capacity: 1\$/capacity: ${capacity_stored}/" | blob)" + gs update-ref refs/locks/sem/legacy/meta "${rewritten}" + out="$(git-locks doctor 2>&1)" + findings n "${out}" sem-record + check "doctor on stored capacity ${capacity_stored} has ${capacity_findings} sem-record findings" "${n}" "${capacity_findings}" + valid "doctor lines on stored capacity ${capacity_stored}" "${out}" +done +gs update-ref refs/locks/sem/legacy/meta "${legacy_meta}" +for i in $(seq 1 9); do + slot="$(printf 'schema: git-locks-slot/1\nsemaphore: legacy\njob: l%s\nholder: bob\nclaimed: 1000000\nexpires: 2000000\nacquisition: t-l%s' "${i}" "${i}" | blob)" + gs update-ref "refs/locks/sem/legacy/slots/l${i}" "${slot}" +done +rewritten="$(gs cat-file -p "${legacy_meta}" | sed 's/^capacity: 1$/capacity: 010/' | blob)" +gs update-ref refs/locks/sem/legacy/meta "${rewritten}" +out="$(git-locks doctor 2>&1)" +findings n "${out}" sem-capacity +check "nine live slots under stored capacity 010 (ten, not octal eight) are not a sem-capacity finding" "${n}" "0" +for i in $(seq 1 9); do gs update-ref -d "refs/locks/sem/legacy/slots/l${i}"; done +gs update-ref refs/locks/sem/legacy/meta "${legacy_meta}" +git-locks sem delete legacy >/dev/null 2>&1 + # An unreadable store is an error, never healthy. err="$(PATH="${BROKEN}:${PATH}" git-locks doctor 2>&1 >/dev/null)" rc="$?"