Skip to content

Wait for clamd's finish script before restarting it in the reload test - #101

Merged
ksylvan merged 1 commit into
mailserver2:masterfrom
AndrewSav:test-clamd-restart-race
Nov 14, 2026
Merged

ksylvan merged 1 commit into
mailserver2:masterfrom
AndrewSav:test-clamd-restart-race

Conversation

@AndrewSav

Copy link
Copy Markdown
Collaborator

Description

Note

TLDR: another small fix for race in tests that sometimes makes tests fail for no good reason.

  • What is the current behavior (you can also link to an open issue here) ?

    The test restarts clamd before running the reload:

    s6-svc -d /services/clamd; while pgrep -x clamd > /dev/null; do sleep 0.2; done; s6-svc -u /services/clamd

    setup.sh gives every service a finish script that runs s6-svc -d when the service exits with code 0, so that a service stopped on purpose stays down. That script runs after the clamd process has exited, so the loop above can finish before it does. When the test's -u lands first, the finish script's -d wins and clamd stays down. The reload's ping (clamdscan --ping 300:1) then gives up after 300 seconds and the test fails.

    It is rare: one failure in a day of local suite runs, and none in 21 loops of the exact sequence. In those loops the finish script logged 32 to 196 ms before the test's -u. The failed run ended with clamd down (exitcode 0) and the finish script's stopping clamd process in the log.

    The image itself does not hit this. It brings clamd up from freshclam's run script and reloads it with SIGUSR2; nothing in it stops clamd and starts it again.

  • What is the new behavior (if this is a feature change) ?

    s6-svc -wD -d /services/clamd; s6-svc -u /services/clamd

    -wD returns only once the service is down and its finish script has exited, so the -u can no longer be undone. The image's own .s6-svscan/finish waits the same way. No other test stops and restarts a service.

Type of change

  • Test (adding missing tests or correcting existing ones)

Status

  • Ready

Todo List

  • Tests

How has this been tested ?

  • With a 1-second delay added to clamd's finish script before its -d, the old sequence left clamd down 3 times out of 3, the same end state as the failed run. With the same delay in place, the new sequence brought clamd up 3 times out of 3.
  • The test still catches the bug it was written for: straight after the new restart, a reload that does not wait for clamd fails with rc 2 (socket missing).
  • The default suite passed 136/136 with this change, on 9d2d46a. The branch has since been rebased onto bd09d3b, which only touches the workflows; the change itself is identical.

The "cron user's reload waits for clamd to start" test stopped clamd and
sent s6-svc -u as soon as the clamd process was gone. On exit code 0,
clamd's finish script sends its own s6-svc -d, and that runs after the
process exits. When the test's -u landed before it, clamd stayed down,
the reload's ping timed out, and the test failed with "Could not connect
to clamd". s6-svc -wD -d returns only once the finish script has exited.
@ksylvan
ksylvan merged commit 38c6cd3 into mailserver2:master Nov 14, 2026
9 checks passed
@AndrewSav
AndrewSav deleted the test-clamd-restart-race branch November 14, 2026 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants