con / con/git-annex

WebDAV remote hangs permanently after credential error (regression in 10.20260901)

Open
#290 0 comments 0 reactions 0 assignees View on GitHub
ai-uncurated bug git-annex regression severity-important
Dominant language
Python
Stars
29
Forks
5
PR merge metrics
No merged PRs in 30d

Description

## WebDAV remote hangs permanently after credential error (regression in 10.20260901)

After a WebDAV remote encounters a credential error, every subsequent WebDAV
operation in the **same process** hangs indefinitely instead of reporting the
error and continuing. A `git annex copy --to=webdav file1 file2` with two
files will fail file1 immediately, then block forever on file2.

**Introduced by**: [10.20260717-26-gc2bdadd0cb AKA 10.20260901~62](https://github.com/con/git-annex/commit/c2bdadd0cb80f830a2f80ab1cea0db636b457a8d)

### Root cause

`Remote/WebDAV.hs` — `withDavHandle` was changed from `TVar` to `TMVar` in
commit `c2bdadd0cb`. The new implementation calls `atomically (takeTMVar hv)`
before running the initialization closure (`mkhdl`). If `mkhdl` throws a
Haskell exception — specifically `giveup "bad creds"` from the credential
decoding path — `putTMVar` is never reached and the `TMVar` stays empty for
the lifetime of the process.

```haskell
-- Remote/WebDAV.hs (10.20260901) — withDavHandle initialization branch
Left mkhdl -> do
hdl <- mkhdl -- TMVar is EMPTY here
liftIO $ atomically $ putTMVar hv (Right hdl) -- never called on exception
either giveup a hdl
```

**Trigger condition — malformed embedded credentials, not absent ones.**
If the WebDAV remote has no credentials configured at all, `getCreds` returns
`Nothing` without throwing, and `mkDavHandleVar`'s closure returns
`Left "webdav credentials not available"` — the `putTMVar` still runs and
no deadlock occurs. The deadlock requires credentials that are *present but
malformed*: a `davcreds` field in the git-annex branch whose base64-decoded
value does not split into exactly two lines (username / password). In that
case `decodeCredPair` returns `Nothing` and `getRemoteCredPair`'s `fromcreds`
calls `giveup "bad creds"` (`Creds.hs:161`), throwing `ErrorCall` that escapes
the `mkhdl` closure with the `TMVar` empty.

Exception path:
```
mkhdl → getCreds → getRemoteCredPairFor → getRemoteCredPair →
fromcreds → decodeCredPair returns Nothing → giveup "bad creds" (ErrorCall)
```

The exception is caught by `accountCommandAction`'s `tryNonAsync`, which logs
the error and moves to the next file. That next file calls `withDavHandle`
again, hits `atomically (takeTMVar hv)` on the now-empty `TMVar`, and blocks
forever.

The old `TVar`-based implementation was immune: `readTVarIO` is non-destructive
and the state persists unchanged across exceptions.

**Additional behavioural regression (concurrent operations).** Even when
credentials are valid, the `TMVar` approach serializes all `withDavHandle`
callers: every call takes the `TMVar`, immediately re-puts it, then calls the
action. The old `TVar` `readTVarIO` was non-blocking and non-exclusive, allowing
genuine concurrent WebDAV operations. The comment added by this commit —
"Concurrent actions are allowed to run at the same time with the same
DavHandle, so any use of eg setDepth will affect other actions" — was intended
to document why concurrency is permitted, but the `TMVar` implementation now
serializes callers. Fixing the deadlock (see below) should also restore
non-serializing access for the initialized case.

### Proposed fix

Restore the `TMVar` to `Left mkhdl` if `mkhdl` throws, so subsequent callers
can retry or report a clean error:

```haskell
Left mkhdl -> do
hdl <- mkhdl `onException`
liftIO (atomically (putTMVar hv (Left mkhdl)))
liftIO $ atomically $ putTMVar hv (Right hdl)
either giveup a hdl
```

Reproducer (POSIX shell, exits 1 when bug fires)

```sh
#!/bin/sh
# Reproducer for: WebDAV withDavHandle TMVar deadlock on initialization exception
#
# Root cause (Remote/WebDAV.hs, withDavHandle, commit c2bdadd0cb):
# DavHandleVar was changed from TVar to TMVar. In the "Left mkhdl" branch,
# takeTMVar is called first. If mkhdl throws a Haskell exception (ErrorCall
# via giveup), putTMVar is never called. Every subsequent withDavHandle call
# on the same remote in the same process blocks forever on takeTMVar.
#
# Trigger path:
# getCreds -> getRemoteCredPairFor -> getRemoteCredPair -> fromconfig ->
# fromcreds -> decodeCredPair returns Nothing -> giveup "bad creds"
#
# Observable symptom:
# With >=2 files passed to "git annex copy --to=" in a single
# process, the first file triggers giveup (caught, reported as error),
# and the second file hangs indefinitely on takeTMVar.
# timeout(1) returns exit 124 (timed out) confirming the hang.
#
# Affected: git-annex 10.20260901 (introduced by TMVar change vs prior TVar)
# Fixed in: unfixed as of reproducer creation date

set -eux
PS4='> '

WORK=$(mktemp -d "${TMPDIR:-/tmp}/ga-webdav-deadlock-XXXXXXX")
cd "$WORK"

GA="${GA:-git-annex}"
if [ -x /tmp/ga-10260901/usr/bin/git-annex ]; then
GA=/tmp/ga-10260901/usr/bin/git-annex
fi

echo "git-annex version:"
"$GA" version | head -1

git init
"$GA" init

python3 - <<'PYEOF' &
from http.server import HTTPServer, BaseHTTPRequestHandler

class MinimalDavHandler(BaseHTTPRequestHandler):
def do_PUT(self):
n = int(self.headers.get('Content-Length', 0))
self.rfile.read(n)
self.send_response(201)
self.end_headers()
def do_DELETE(self):
self.send_response(204)
self.end_headers()
def do_MKCOL(self):
self.send_response(201)
self.end_headers()
def log_message(self, *args): pass

HTTPServer(('127.0.0.1', 19998), MinimalDavHandler).serve_forever()
PYEOF
WEBDAV_SERVER_PID=$!

_i=0
while [ "$_i" -lt 50 ]; do
python3 -c "import socket; s=socket.socket(); s.connect(('127.0.0.1',19998)); s.close()" 2>/dev/null && break
_i=$(( _i + 1 ))
sleep 0.1
done
unset _i

WEBDAV_USERNAME=gooduser WEBDAV_PASSWORD=goodpass \
"$GA" initremote webdav \
type=webdav \
url=http://127.0.0.1:19998/ \
encryption=none \
embedcreds=yes

kill "$WEBDAV_SERVER_PID" 2>/dev/null || true
wait "$WEBDAV_SERVER_PID" 2>/dev/null || true

WEBDAV_UUID=$(git config remote.webdav.annex-uuid)
BAD_B64="YmFk"
TIMESTAMP=$(date +%s)

CURR_LOG=$(git show git-annex:remote.log)
NEW_LOG=$(printf '%s\n' "$CURR_LOG" | \
sed "s|^${WEBDAV_UUID} .*|${WEBDAV_UUID} davcreds=${BAD_B64} embedcreds=yes encryption=none name=webdav type=webdav url=http://127.0.0.1:19998/ timestamp=${TIMESTAMP}s|")

BAD_BLOB=$(printf '%s\n' "$NEW_LOG" | git hash-object -w --stdin)
NEW_TREE=$(
{ git ls-tree git-annex | grep -v ' remote\.log$'
printf '100644 blob %s\tremote.log\n' "$BAD_BLOB"
} | git mktree
)
NEW_COMMIT=$(git commit-tree "$NEW_TREE" -p git-annex -m "corrupt webdav davcreds for deadlock test")
git update-ref refs/heads/git-annex "$NEW_COMMIT"
rm -f ".git/annex/creds/${WEBDAV_UUID}"
"$GA" merge

printf 'alpha content\n' > file_alpha.txt
printf 'beta content\n' > file_beta.txt
"$GA" add file_alpha.txt file_beta.txt
git commit -m "add two test files"

echo ""
echo "--- single-file copy (no deadlock expected): ---"
set +e
"$GA" copy --to=webdav file_alpha.txt
SINGLE_EXIT=$?
set -e
echo "single-file exit: $SINGLE_EXIT"
if [ "$SINGLE_EXIT" -eq 0 ]; then
echo "UNEXPECTED: single-file copy succeeded with bad credentials"
exit 1
fi

rm -f ".git/annex/creds/${WEBDAV_UUID}"

echo ""
echo "--- two-file copy with timeout 10s (deadlock expected): ---"
set +e
timeout 10 "$GA" copy --to=webdav file_alpha.txt file_beta.txt
TWO_EXIT=$?
set -e
echo "two-file exit: $TWO_EXIT"

if [ "$TWO_EXIT" -eq 124 ]; then
echo "BUG CONFIRMED: second takeTMVar blocked for >10s (TMVar was never restored)"
echo "Affected version: $("$GA" version | head -1)"
exit 1
elif [ "$TWO_EXIT" -eq 0 ]; then
echo "UNEXPECTED: two-file copy succeeded with bad credentials"
exit 2
else
echo "Two-file copy failed with exit $TWO_EXIT (not a deadlock -- may be fixed)"
exit 0
fi
```

Notes:
- Verified to exit 1 (deadlock confirmed) on 10.20260901.
- The stub WebDAV server is only needed for `initremote`; the actual bug fires
with the server offline (it's the credential decode, not the network, that
throws).
- Requires `git`, `python3`, and `timeout` (GNU coreutils).

Regression evidence

The introducing commit is [10.20260717-26-gc2bdadd0cb AKA 10.20260901~62](https://github.com/con/git-annex/commit/c2bdadd0cb80f830a2f80ab1cea0db636b457a8d).
The parent commit had `TVar`-based `withDavHandle` which is immune: `readTVarIO`
is non-destructive so a failed `mkhdl` left the `TVar` in its `Left mkhdl`
state, allowing subsequent calls to retry or immediately re-report the error.

The commit message and CHANGELOG do not mention any change in error-handling
behaviour for WebDAV credential failures — the intent was to serialise
initialization. The deadlock is an unintended side effect.

Additionally: the comment added by the commit — "Concurrent actions are allowed
to run at the same time with the same DavHandle, so any use of eg setDepth will
affect other actions" — is now misleading, since the `TMVar` approach briefly
serialises access during initialization in a way the `TVar` did not.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in Remote/WebDAV.hs at withDavHandle and review the TMVar initialization path, then inspect the credential failure path through Creds.hs:161. Run the supplied POSIX reproducer to confirm the hang. Done means malformed credentials no longer leave later operations blocked, and valid concurrent WebDAV actions are not unnecessarily serialized.

Written by the indexing model from the issue text.

Assessment

Tech stack
git, haskell
Domain
cli, networking
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.