Repository navigation
conmon: do not fire --timeout early - #708
Merged
Merged
Conversation
jnovy
approved these changes
Oct 26, 2026
jnovy
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, nice fix.
The overflow guard (guint)opt_timeout <= G_MAXUINT / 1000 is correct - max product is 4294967000 which fits in guint. The (guint) cast is safe because the outer opt_timeout > 0 check guarantees a positive value (opt_timeout is int per cli.c:42). Fallback to g_timeout_add_seconds() for >49-day timeouts is a sensible choice since 250ms imprecision is irrelevant at that scale.
Builds clean with -Werror locally, no warnings.
One note: the PR branch needs a rebase onto current main - the diff includes stale GitHub Actions version pins from before renovate PRs #700, #701, #704 merged. Without rebase, merging would revert those updates.
The --timeout timer is set with g_timeout_add_seconds, which rounds the expiration time to a whole second boundary (adjusted by a per-host offset) to group wakeups. When the sub-second remainder is less than 250ms it is rounded down, so the timer can fire up to 250ms before the requested timeout. As a result, "podman run --timeout 2 IMAGE sleep 60" sometimes takes less than 2 seconds in total (as measured locally, 1976-2019ms over 12 runs, with about 100ms spent on container setup), and podman's "podman run --timeout - basic test" system test flakes with: FAIL: podman stop: ran too quickly! expected: -gt 1 actual: 1 Use g_timeout_add with milliseconds instead, which does not round. With this change, the same command takes 2103-2123ms. Keep g_timeout_add_seconds for timeouts that do not fit into guint milliseconds (about 49 days), as the imprecision does not matter there. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin
force-pushed
the
kir/timeout-precise
branch
from
October 26, 2026 09:24
d717898 to
7f06cc8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
--timeouttimer is set withg_timeout_add_seconds, which rounds the expiration time to a whole second boundary (adjusted by a per-host offset) to group wakeups. When the sub-second remainder is less than 250ms it is rounded down, so the timer can fire up to 250ms before the requested timeout (seeg_timeout_set_expirationin glib'sgmain.c).As a result,
podman run --timeout 2 IMAGE sleep 60sometimes takes less than 2 seconds in total, and podman'spodman run --timeout - basic testsystem test flakes with:(seen in runc CI: https://github.com/opencontainers/runc/actions/runs/40177692604/job/121122129191, where the whole
podman runtook 1.92s).Use
g_timeout_addwith milliseconds instead, which does not round. Keepg_timeout_add_secondsfor timeouts that do not fit intoguintmilliseconds (about 49 days), as the imprecision does not matter there.Measured locally (rootless podman 5.8.9, 12 runs of
podman run --rm --timeout 2 ... sleep 60, container setup takes ~100ms):