diff --git a/bin/omarchy-menu-images b/bin/omarchy-menu-images index ddf34c25..8c316462 100755 --- a/bin/omarchy-menu-images +++ b/bin/omarchy-menu-images @@ -111,8 +111,8 @@ cache_key=$(printf '%s' "$image_dirs_env" | md5sum | cut -d ' ' -f 1) rows_cache_file="$cache_dir/$cache_key.rows" rows_signature_file="$cache_dir/$cache_key.signature" rows_fast_signature_file="$cache_dir/$cache_key.fast-signature" -rows_signature="v2"$'\n' -rows_fast_signature="v1"$'\n' +rows_signature="v3"$'\n' +rows_fast_signature="v2"$'\n' rows_cacheable=true rows_cache_hit=false image_files=() @@ -142,23 +142,32 @@ generate_thumbnail() { local image="$1" local thumbnail="$2" local lock="$thumbnail.lock" + local lock_fd local tmp="$thumbnail.$$.jpg" - if mkdir "$lock" 2>/dev/null; then - # Callers fan out one generator per image, so keep each vips single-threaded. - if VIPS_CONCURRENCY=1 vipsthumbnail "$image" --size 1536x864 --smartcrop=centre --path "$tmp[Q=82,strip]"; then - mv -f "$tmp" "$thumbnail" - else - rm -f "$tmp" "$thumbnail" - fi + # Older releases used directories as locks, which could survive a killed + # generator and block this thumbnail forever. Only reap aged ones, so a + # legacy generator still running through an upgrade keeps its lock. + if [[ -d $lock ]] && (( $(date +%s) - $(stat -c '%Y' "$lock" 2>/dev/null || date +%s) > 120 )); then + rmdir "$lock" 2>/dev/null + fi - rmdir "$lock" 2>/dev/null || true + exec {lock_fd}>"$lock" || return + flock -w 30 "$lock_fd" || return + + # A generator killed mid-write leaves its partial $thumbnail..jpg + # behind. Only the lock holder writes these, so any found now are stale. + rm -f "$thumbnail".*.jpg + + [[ -f $thumbnail ]] && return + + # Callers fan out one generator per image, so keep each vips single-threaded. + # Close the lock fd for vips: an orphaned or hung vips must not keep holding + # the lock after this shell is killed. + if VIPS_CONCURRENCY=1 vipsthumbnail "$image" --size 1536x864 --smartcrop=centre --path "$tmp[Q=82,strip]" {lock_fd}>&-; then + mv -f "$tmp" "$thumbnail" else - for ((i = 0; i < 3000; i++)); do - [[ -f $thumbnail ]] && return - [[ -d $lock ]] || break - sleep 0.01 - done + rm -f "$tmp" "$thumbnail" fi } @@ -226,7 +235,10 @@ elif [[ $rows_cache_hit != true ]]; then if [[ -s $pending_file ]]; then pruned="" while IFS=$'\t' read -r row_image row_thumbnail; do - [[ -e $row_thumbnail ]] || continue + if [[ ! -e $row_thumbnail ]]; then + rows_cacheable=false + continue + fi if [[ -z $pruned ]]; then pruned="$row_image"$'\t'"$row_thumbnail" @@ -237,12 +249,21 @@ elif [[ $rows_cache_hit != true ]]; then rows="$pruned" fi - if [[ $rows_cacheable == true ]]; then - printf '%s' "$rows" >"$rows_cache_file" - printf '%s' "$rows_signature" >"$rows_signature_file" - printf '%s' "$rows_fast_signature" >"$rows_fast_signature_file" - else - rm -f "$rows_cache_file" "$rows_signature_file" "$rows_fast_signature_file" + # Publish the cache under a lock and via renames: a picker killed mid-write, + # or two pickers interleaving, must never leave truncated or mismatched rows + # behind signatures that still validate. Rows go first so a kill between + # renames leaves signatures that are either older (a harmless miss) or + # describe the same directory state. + if exec {rows_lock_fd}>"$rows_cache_file.lock" && flock -w 30 "$rows_lock_fd"; then + if [[ $rows_cacheable == true ]]; then + rm -f "$cache_dir/$cache_key".*.tmp + printf '%s' "$rows" >"$rows_cache_file.$$.tmp" && mv -f "$rows_cache_file.$$.tmp" "$rows_cache_file" + printf '%s' "$rows_signature" >"$rows_signature_file.$$.tmp" && mv -f "$rows_signature_file.$$.tmp" "$rows_signature_file" + printf '%s' "$rows_fast_signature" >"$rows_fast_signature_file.$$.tmp" && mv -f "$rows_fast_signature_file.$$.tmp" "$rows_fast_signature_file" + else + rm -f "$rows_cache_file" "$rows_signature_file" "$rows_fast_signature_file" + fi + exec {rows_lock_fd}>&- fi fi diff --git a/test/shell.d/menu-images-test.sh b/test/shell.d/menu-images-test.sh new file mode 100644 index 00000000..ac20a8ca --- /dev/null +++ b/test/shell.d/menu-images-test.sh @@ -0,0 +1,141 @@ +#!/bin/bash + +set -euo pipefail + +source "$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)/base-test.sh" + +require_command flock + +tmp=$(mktemp -d) +trap 'rm -rf "$tmp"' EXIT + +cache_home="$tmp/cache" +images="$tmp/images" +stub_bin="$tmp/bin" +mkdir -p "$images" "$stub_bin" + +cat >"$stub_bin/vipsthumbnail" <<'EOF' +#!/bin/bash + +image="$1" +shift + +while (( $# > 0 )); do + if [[ $1 == "--path" ]]; then + output=${2%%\[*} + break + fi + shift +done + +if [[ -f ${VIPSTHUMBNAIL_FAIL_FILE:-} ]] && grep -Fxq "$image" "$VIPSTHUMBNAIL_FAIL_FILE"; then + exit 1 +fi + +[[ -z ${VIPSTHUMBNAIL_CALLS_FILE:-} ]] || printf '%s\n' "$image" >>"$VIPSTHUMBNAIL_CALLS_FILE" +[[ -z ${VIPSTHUMBNAIL_DELAY:-} ]] || sleep "$VIPSTHUMBNAIL_DELAY" +printf 'thumbnail' >"$output" +EOF +chmod +x "$stub_bin/vipsthumbnail" + +for name in one two three; do + printf 'image-%s' "$name" >"$images/$name.png" +done + +cache_dir="$cache_home/omarchy/image-selector" +mkdir -p "$cache_dir" + +stale_tmp="" +live_lock="" +for image in "$images"/*; do + signature=$(stat -Lc '%s:%Y' "$image") + hash=$(printf '%s\t%s' "$image" "$signature" | md5sum | cut -d ' ' -f 1) + mkdir "$cache_dir/$hash.jpg.lock" + touch -m -d '10 minutes ago' "$cache_dir/$hash.jpg.lock" + stale_tmp="$cache_dir/$hash.jpg.4242.jpg" + live_lock="$cache_dir/$hash.jpg.lock" +done +printf 'partial' >"$stale_tmp" + +cache_key=$(printf '%s' "$images" | md5sum | cut -d ' ' -f 1) +printf '%s\t%s' "$images/one.png" "$cache_dir/missing.jpg" >"$cache_dir/$cache_key.rows" +printf 'v2\n%s:%s\n' "$images" "$(stat -Lc '%Y' "$images")" >"$cache_dir/$cache_key.signature" +printf 'v1\n%s:%s\n' "$images" "$(stat -Lc '%Y' "$images")" >"$cache_dir/$cache_key.fast-signature" + +PATH="$stub_bin:$PATH" XDG_CACHE_HOME="$cache_home" \ + "$ROOT/bin/omarchy-menu-images" --cache-only "$images" + +(( $(find "$cache_dir" -maxdepth 1 -name '*.jpg' -type f | wc -l) == 3 )) || + fail "image menu recovers thumbnails from stranded locks" +(( $(awk 'END { print NR }' "$cache_dir/$cache_key.rows") == 3 )) || + fail "image menu rebuilds every row after cache invalidation" +[[ $(head -n 1 "$cache_dir/$cache_key.signature") == "v3" ]] || + fail "image menu invalidates stale row caches" +[[ ! -e $stale_tmp ]] || + fail "image menu clears partial thumbnails left by killed generators" +pass "image menu recovers stranded locks and stale rows" + +rm -rf "$cache_home" +mkdir -p "$cache_dir" +mkdir "$live_lock" + +PATH="$stub_bin:$PATH" XDG_CACHE_HOME="$cache_home" \ + "$ROOT/bin/omarchy-menu-images" --cache-only "$images" + +(( $(find "$cache_dir" -maxdepth 1 -name '*.jpg' -type f | wc -l) == 2 )) || + fail "image menu skips a thumbnail whose fresh legacy lock may still be owned" +[[ -d $live_lock ]] || + fail "image menu leaves a fresh legacy lock directory alone" +[[ ! -e $cache_dir/$cache_key.rows ]] || + fail "image menu does not cache rows while a legacy generator holds a lock" +pass "image menu respects a live legacy generator's lock" + +rm -rf "$cache_home" +mkdir -p "$cache_home" +printf '%s\n' "$images/two.png" >"$tmp/failures" + +PATH="$stub_bin:$PATH" XDG_CACHE_HOME="$cache_home" VIPSTHUMBNAIL_FAIL_FILE="$tmp/failures" \ + "$ROOT/bin/omarchy-menu-images" --cache-only "$images" + +cache_dir="$cache_home/omarchy/image-selector" +[[ ! -e $cache_dir/$cache_key.rows ]] || fail "image menu does not cache incomplete rows" +[[ ! -e $cache_dir/$cache_key.signature ]] || fail "image menu does not sign incomplete rows" +[[ ! -e $cache_dir/$cache_key.fast-signature ]] || fail "image menu does not fast-cache incomplete rows" +pass "image menu leaves failed thumbnail batches uncached" + +rm "$tmp/failures" +PATH="$stub_bin:$PATH" XDG_CACHE_HOME="$cache_home" \ + "$ROOT/bin/omarchy-menu-images" --cache-only "$images" + +(( $(find "$cache_dir" -maxdepth 1 -name '*.jpg' -type f | wc -l) == 3 )) || + fail "image menu retries a previously failed thumbnail" +(( $(awk 'END { print NR }' "$cache_dir/$cache_key.rows") == 3 )) || + fail "image menu caches every row after retry" +pass "image menu completes and caches a later retry" + +rm -rf "$cache_home" +mkdir -p "$cache_home" +: >"$tmp/calls" + +# The delay keeps both runs inside the generation window so the locks are +# actually contended rather than the second run arriving after the first. +pids=() +for run in 1 2; do + PATH="$stub_bin:$PATH" XDG_CACHE_HOME="$cache_home" \ + VIPSTHUMBNAIL_CALLS_FILE="$tmp/calls" VIPSTHUMBNAIL_DELAY=0.25 \ + "$ROOT/bin/omarchy-menu-images" --cache-only "$images" & + pids+=($!) +done +for pid in "${pids[@]}"; do + wait "$pid" || fail "concurrent image menu runs exit cleanly" +done + +(( $(wc -l <"$tmp/calls") == 3 )) || fail "image menu serializes concurrent thumbnail generators" + +rm -f "$cache_dir"/*.jpg +rm -f "$cache_dir/$cache_key.rows" "$cache_dir/$cache_key.signature" "$cache_dir/$cache_key.fast-signature" +PATH="$stub_bin:$PATH" XDG_CACHE_HOME="$cache_home" VIPSTHUMBNAIL_CALLS_FILE="$tmp/calls" \ + "$ROOT/bin/omarchy-menu-images" --cache-only "$images" + +(( $(wc -l <"$tmp/calls") == 6 )) || fail "image menu releases thumbnail locks after generation" +pass "image menu owns locks for exactly one generator lifetime"