Skip to content

Commit 364dcc5

Browse files
committed
t/README: document writing concurrency-safe helpers
The two preceding fixes addressed the same underlying problem: a test helper assuming it has exclusive access to a file when the web server can run it for several requests at once. The atomic idioms that avoid this are not specific to CGI or to HTTP, so document them generally, alongside the other guidance for writing tests, and leave a pointer from the lib-httpd helper list rather than a local comment. The note covers the anti-pattern (a "test -f" then a separate act) and the two safe operations (mkdir to elect a winner, rename to consume a one-shot marker), citing Git's own lockfile.c and make_symlink() as precedent. Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
1 parent 7b8759a commit 364dcc5

2 files changed

Lines changed: 35 additions & 0 deletions

File tree

t/README

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -854,6 +854,38 @@ from the test harness library. At the end of the script, call
854854
'test_done'.
855855

856856

857+
Writing concurrency-safe helpers
858+
--------------------------------
859+
860+
Some test code runs concurrently: a test may background work with '&',
861+
and the helper scripts installed for the web server (in t/lib-httpd) or
862+
for git-daemon are run once per request, so the same script can execute
863+
for several requests at once. Such code cannot assume it has exclusive
864+
access to a file.
865+
866+
When exactly one of several concurrent processes needs to "win" a
867+
decision, a single atomic filesystem operation can make it, rather than
868+
a check followed by a separate action. A "test -f X" then "touch X"
869+
(or "rm X") races: two processes can both pass the check before either
870+
acts. Two atomic operations avoid this:
871+
872+
- "mkdir dir", which fails if the directory already exists, so that
873+
exactly one caller wins, electing a first or only request (see
874+
t/lib-httpd/http-429.sh).
875+
876+
- "mv src dst" (rename), which fails if the source is gone, so that
877+
exactly one caller consumes it, claiming a planted one-shot marker
878+
(see t/lib-httpd/apply-one-time-script.sh).
879+
880+
A "$$" suffix on per-request scratch files keeps concurrent invocations
881+
from clobbering each other's fixed-name files.
882+
883+
This is a standard shell locking idiom, and the same reasoning behind
884+
Git's own lockfile.c, which uses O_CREAT|O_EXCL, and make_symlink() in
885+
t/test-lib.sh, which uses an mkdir lock: an atomic operation whose
886+
failure indicates that another process got there first.
887+
888+
857889
Test harness library
858890
--------------------
859891

t/lib-httpd.sh

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,9 @@ prepare_httpd() {
159159
mkdir -p "$HTTPD_DOCUMENT_ROOT_PATH"
160160
cp "$TEST_PATH"/passwd "$HTTPD_ROOT_PATH"
161161
cp "$TEST_PATH"/proxy-passwd "$HTTPD_ROOT_PATH"
162+
# The web server can run any of these CGI scripts for two requests at
163+
# once; a helper that keeps state between requests must do so with an
164+
# atomic operation. See "Writing concurrency-safe helpers" in t/README.
162165
install_script incomplete-length-upload-pack-v2-http.sh
163166
install_script incomplete-body-upload-pack-v2-http.sh
164167
install_script error-no-report.sh

0 commit comments

Comments
 (0)