Fix frame rate limiting blocking forever when the clock cannot observe sleep

The frame rate limiting loop in flecs_insert_sleep slept a fixed
interval and remeasured until the frame reached the target time. When
the clock does not advance during a sleep, the loop never exits: a clock
that is only stepped by the host in between frames (emscripten without
asyncify, simulated clocks) blocks ecs_progress forever as soon as a
target FPS is set. A clock too coarse to observe an individual sleep
does exit, but only by spinning through many sleep and measure calls per
clock tick.

Sleep intervals now escalate when the clock is not keeping up. The clock
counts as keeping up when it advances by at least a quarter of the
interval just slept, which measures whether get_time and sleep agree
about the passage of time rather than how fast the clock runs, so a host
that virtualizes both consistently still paces correctly at any speed.
On a coarse clock, escalation grows the interval until it crosses a
clock tick, which keeps pacing working at reduced granularity. On a
clock that does not advance at all, two bounds end the frame: a stall
budget of one frame period that never refills within a frame, and an
unconditional cap of two frame periods on the total sleep requested per
frame. The budget not refilling matters, because a budget that resets
whenever the clock advances a little lets clock jitter replay the
escalation chain indefinitely on a clock that advances just enough to be
counted once per chain. The cap is denominated in requested sleep while
pacing is measured on the clock, so a clock that observes less than
about half of each sleep ends its frame proportionally short of the
target rather than sleeping longer. The ecs_set_target_fps documentation
states this contract, including the shortfall.

ecs_set_target_fps also rejects targets that are not zero or within
1e-9 to 1e9. The frame time is the reciprocal of the target, so a
subnormal target yields an infinite frame time and sleep interval, and
the value can arrive from outside the application through the REST world
summary endpoint; the range keeps the frame time finite and within
[1e-9, 1e9] by construction, rejects NaN through the comparison, and
compiles for any type ecs_ftime_t is defined to, including the integer
configuration in the custom build tests. In release builds the check
compiles out and is backstopped by ecs_sleepf, which now refuses
durations it cannot convert to the seconds and nanoseconds the OS sleep
API takes: converting a value of 2^31 seconds or more to int is
undefined behavior, and the same comparison rejects NaN. A refused sleep
returns immediately, is measured as the clock not keeping up, and rate
limiting is skipped for the frame instead of waiting.

Tests cover a stalled clock with a target FPS set, with both real and
no-op sleep functions; pacing accuracy against a sleep-driven simulated
clock at three work levels; time conservation on a coarse clock with a
target FPS; clocks just above and just below the keeping-up threshold,
which also pin that the cap ends a frame that cannot reach the target;
dither and host-stepped clocks, which bound the total sleep requested
per frame; the fps argument checks; and that ecs_sleepf never forwards
an unrepresentable duration.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: Simon

distr/flecs.c

@@ -13362,7 +13362,11 @@ ecs_time_t ecs_time_sub(
 void ecs_sleepf(
     double t)
 {
-    if (t > 0) {
+    /* Only sleep for a duration that can be represented. Converting a value of
+     * 2^31 seconds or more to int is undefined behavior (out-of-range
+     * conversions produce platform-dependent garbage durations), and the
+     * comparison also rejects NaN. */
+    if (t > 0 && t <= (double)INT32_MAX) {
         int sec = (int)t;
         int nsec = (int)((t - sec) * 1000000000);
         ecs_os_sleep(sec, nsec);
@@ -24373,15 +24377,97 @@ static ecs_ftime_t flecs_insert_sleep(
         sleep_time = 0;
         delta_time = (ecs_ftime_t)ecs_time_measure(&now);
     } else {
+        /* Sleep interval while the clock keeps up, the largest delta time
+         * measured so far, interval slept while the clock did not keep up, and
+         * interval slept for this frame in total. */
+        ecs_ftime_t initial_sleep_time = sleep_time;
+        ecs_ftime_t max_delta_time = delta_time;
+        ecs_ftime_t stalled_sleep = 0;
+        ecs_ftime_t total_sleep = 0;
+
         do {
+            /* Hard bound on how long a single frame may spend sleeping, no
+             * matter what the classification below makes of the clock. The
+             * stall budget is the semantic bound; this one is unconditional, so
+             * that no interaction between the two can exceed what the frame API
+             * documents. */
+            ecs_ftime_t sleep_cap =
+                (target_delta_time * (ecs_ftime_t)2.0) - total_sleep;
+            if (sleep_cap <= 0) {
+                break;
+            }
+            if (sleep_time > sleep_cap) {
+                sleep_time = sleep_cap;
+            }
+
             /* Only call sleep when sleep_time is not 0. On some platforms, even
             * a sleep with a timeout of 0 can cause stutter. */
             if (ECS_NEQZERO(sleep_time)) {
                 ecs_sleepf((double)sleep_time);
+                total_sleep += sleep_time;
             }
 
             now = start;
             delta_time = (ecs_ftime_t)ecs_time_measure(&now);
+
+            /* The clock only counts as keeping up if it advanced by a
+             * meaningful fraction of the interval that was just slept. Testing
+             * for any advance would let a clock that ticks a nanosecond per
+             * read reset the budget below forever while making no progress
+             * towards the target. The ratio measures whether clock and sleep
+             * agree about the passage of time, not how fast the clock runs, so
+             * a host that virtualizes both consistently paces correctly. */
+            if ((delta_time - max_delta_time) >=
+                (sleep_time / (ecs_ftime_t)4.0))
+            {
+                max_delta_time = delta_time;
+
+                /* Restore the interval. The loop's exit condition is expressed
+                 * in terms of it, so leaving it escalated would overshoot the
+                 * target frame time for the rest of the frame. */
+                sleep_time = initial_sleep_time;
+            } else {
+                /* The clock did not keep up: it is either too coarse to
+                 * measure an interval this small, or not running at all (a
+                 * simulated clock that a host advances in between frames). A
+                 * longer sleep eventually crosses a tick on a coarse clock; a
+                 * stopped clock never advances however long is slept. What
+                 * accumulates is therefore the requested interval, not time
+                 * measured by the clock: neither the clock nor a replaced sleep
+                 * function need make progress. A sleep that never returns is a
+                 * broken OS API, out of scope here. */
+                stalled_sleep += sleep_time;
+
+                /* Never spend more than one frame's worth of sleeping on a
+                 * clock that is not keeping up. The budget is per frame and
+                 * never refills: an interval that the clock did keep up with
+                 * restores the sleep interval, but not this. Refilling it lets
+                 * a clock that advances just enough to be counted, once per
+                 * escalation chain, replay that chain for as long as the frame
+                 * lasts. */
+                ecs_ftime_t sleep_budget = target_delta_time - stalled_sleep;
+                if (sleep_budget <= 0) {
+                    break;
+                }
+
+                if (ECS_EQZERO(sleep_time)) {
+                    /* Doubling zero stays zero, so seed the escalation with a
+                     * known nonzero interval (the initial one can be zero, when
+                     * the frame took exactly the target time). A negative zero
+                     * isn't matched by the bitwise test, but it never reaches
+                     * here: an advance of zero is not below a threshold of
+                     * negative zero, so the branch above claims it and restores
+                     * the interval. A frame that took exactly the target time
+                     * exits on the condition below. */
+                    sleep_time = target_delta_time / (ecs_ftime_t)8.0;
+                } else {
+                    sleep_time *= (ecs_ftime_t)2.0;
+                }
+
+                if (sleep_time > sleep_budget) {
+                    sleep_time = sleep_budget;
+                }
+            }
         } while ((target_delta_time - delta_time) >
             (sleep_time / (ecs_ftime_t)2.0));
     }
@@ -24598,6 +24684,18 @@ void ecs_set_target_fps(
     flecs_poly_assert(world, ecs_world_t);
     ecs_check(ecs_os_has_time(), ECS_MISSING_OS_API, NULL);
 
+    /* Frame time is the reciprocal of the target, so targets outside of this
+     * range yield a frame time that is infinite (the reciprocal of a subnormal
+     * target overflows) or otherwise meaningless. The comparisons also reject
+     * NaN, and compile for any type ecs_ftime_t is defined to. In release
+     * builds this check is compiled out, and is backstopped by ecs_sleepf
+     * refusing a duration it cannot represent: the sleep returns immediately,
+     * the clock is measured as not keeping up, and frame rate limiting is
+     * skipped for the frame instead of waiting. */
+    ecs_check(ECS_EQZERO(fps) || (fps >= (ecs_ftime_t)1e-9 &&
+        fps <= (ecs_ftime_t)1e9),
+        ECS_INVALID_PARAMETER, "fps must be zero or in the range 1e-9 to 1e9");
+
     ecs_measure_frame_time(world, true);
     world->info.target_fps = fps;
 error:

distr/flecs.h

@@ -13171,6 +13171,22 @@ void ecs_measure_system_time(
  * Note that ecs_progress() only sleeps if there is time left in the frame. Both
  * time spent in Flecs and time spent outside of Flecs are taken into account.
  *
+ * Frame rate limiting requires the OS API get_time and sleep functions to agree
+ * about the passage of time. Both may run on a virtualized timeline of any
+ * speed, but an application that replaces one of them must replace the other
+ * consistently. The total sleep requested for a single frame is capped at two
+ * frame periods, so a frame never waits for the clock indefinitely. The cap is
+ * measured in requested sleep, so a clock that observes less than about half
+ * of each sleep ends the frame short of the target, in proportion to the
+ * disagreement. If clock and sleep disagree by more than roughly a factor of
+ * four (for example a clock that is only stepped by a host in between frames,
+ * while sleeping does take time) frame rate limiting requests about one frame
+ * period worth of sleep and is then skipped for the remainder of that frame.
+ * If the clock is too coarse to observe an individual sleep, sleep intervals
+ * are enlarged until it can, which makes pacing coarser but keeps it working.
+ * An application that computes its target from untrusted input should range
+ * check it before passing it in.
+ *
  * @param world The world.
  * @param fps The target FPS.
  */

distr/flecs_no_addons.c

@@ -11380,7 +11380,11 @@ ecs_time_t ecs_time_sub(
 void ecs_sleepf(
     double t)
 {
-    if (t > 0) {
+    /* Only sleep for a duration that can be represented. Converting a value of
+     * 2^31 seconds or more to int is undefined behavior (out-of-range
+     * conversions produce platform-dependent garbage durations), and the
+     * comparison also rejects NaN. */
+    if (t > 0 && t <= (double)INT32_MAX) {
         int sec = (int)t;
         int nsec = (int)((t - sec) * 1000000000);
         ecs_os_sleep(sec, nsec);

include/flecs/addons/frame.h

@@ -138,6 +138,22 @@ void ecs_measure_system_time(
  * Note that ecs_progress() only sleeps if there is time left in the frame. Both
  * time spent in Flecs and time spent outside of Flecs are taken into account.
  *
+ * Frame rate limiting requires the OS API get_time and sleep functions to agree
+ * about the passage of time. Both may run on a virtualized timeline of any
+ * speed, but an application that replaces one of them must replace the other
+ * consistently. The total sleep requested for a single frame is capped at two
+ * frame periods, so a frame never waits for the clock indefinitely. The cap is
+ * measured in requested sleep, so a clock that observes less than about half
+ * of each sleep ends the frame short of the target, in proportion to the
+ * disagreement. If clock and sleep disagree by more than roughly a factor of
+ * four (for example a clock that is only stepped by a host in between frames,
+ * while sleeping does take time) frame rate limiting requests about one frame
+ * period worth of sleep and is then skipped for the remainder of that frame.
+ * If the clock is too coarse to observe an individual sleep, sleep intervals
+ * are enlarged until it can, which makes pacing coarser but keeps it working.
+ * An application that computes its target from untrusted input should range
+ * check it before passing it in.
+ *
  * @param world The world.
  * @param fps The target FPS.
  */

src/addons/frame.c

@@ -36,15 +36,97 @@ static ecs_ftime_t flecs_insert_sleep(
         sleep_time = 0;
         delta_time = (ecs_ftime_t)ecs_time_measure(&now);
     } else {
+        /* Sleep interval while the clock keeps up, the largest delta time
+         * measured so far, interval slept while the clock did not keep up, and
+         * interval slept for this frame in total. */
+        ecs_ftime_t initial_sleep_time = sleep_time;
+        ecs_ftime_t max_delta_time = delta_time;
+        ecs_ftime_t stalled_sleep = 0;
+        ecs_ftime_t total_sleep = 0;
+
         do {
+            /* Hard bound on how long a single frame may spend sleeping, no
+             * matter what the classification below makes of the clock. The
+             * stall budget is the semantic bound; this one is unconditional, so
+             * that no interaction between the two can exceed what the frame API
+             * documents. */
+            ecs_ftime_t sleep_cap =
+                (target_delta_time * (ecs_ftime_t)2.0) - total_sleep;
+            if (sleep_cap <= 0) {
+                break;
+            }
+            if (sleep_time > sleep_cap) {
+                sleep_time = sleep_cap;
+            }
+
             /* Only call sleep when sleep_time is not 0. On some platforms, even
             * a sleep with a timeout of 0 can cause stutter. */
             if (ECS_NEQZERO(sleep_time)) {
                 ecs_sleepf((double)sleep_time);
+                total_sleep += sleep_time;
             }
 
             now = start;
             delta_time = (ecs_ftime_t)ecs_time_measure(&now);
+
+            /* The clock only counts as keeping up if it advanced by a
+             * meaningful fraction of the interval that was just slept. Testing
+             * for any advance would let a clock that ticks a nanosecond per
+             * read reset the budget below forever while making no progress
+             * towards the target. The ratio measures whether clock and sleep
+             * agree about the passage of time, not how fast the clock runs, so
+             * a host that virtualizes both consistently paces correctly. */
+            if ((delta_time - max_delta_time) >=
+                (sleep_time / (ecs_ftime_t)4.0))
+            {
+                max_delta_time = delta_time;
+
+                /* Restore the interval. The loop's exit condition is expressed
+                 * in terms of it, so leaving it escalated would overshoot the
+                 * target frame time for the rest of the frame. */
+                sleep_time = initial_sleep_time;
+            } else {
+                /* The clock did not keep up: it is either too coarse to
+                 * measure an interval this small, or not running at all (a
+                 * simulated clock that a host advances in between frames). A
+                 * longer sleep eventually crosses a tick on a coarse clock; a
+                 * stopped clock never advances however long is slept. What
+                 * accumulates is therefore the requested interval, not time
+                 * measured by the clock: neither the clock nor a replaced sleep
+                 * function need make progress. A sleep that never returns is a
+                 * broken OS API, out of scope here. */
+                stalled_sleep += sleep_time;
+
+                /* Never spend more than one frame's worth of sleeping on a
+                 * clock that is not keeping up. The budget is per frame and
+                 * never refills: an interval that the clock did keep up with
+                 * restores the sleep interval, but not this. Refilling it lets
+                 * a clock that advances just enough to be counted, once per
+                 * escalation chain, replay that chain for as long as the frame
+                 * lasts. */
+                ecs_ftime_t sleep_budget = target_delta_time - stalled_sleep;
+                if (sleep_budget <= 0) {
+                    break;
+                }
+
+                if (ECS_EQZERO(sleep_time)) {
+                    /* Doubling zero stays zero, so seed the escalation with a
+                     * known nonzero interval (the initial one can be zero, when
+                     * the frame took exactly the target time). A negative zero
+                     * isn't matched by the bitwise test, but it never reaches
+                     * here: an advance of zero is not below a threshold of
+                     * negative zero, so the branch above claims it and restores
+                     * the interval. A frame that took exactly the target time
+                     * exits on the condition below. */
+                    sleep_time = target_delta_time / (ecs_ftime_t)8.0;
+                } else {
+                    sleep_time *= (ecs_ftime_t)2.0;
+                }
+
+                if (sleep_time > sleep_budget) {
+                    sleep_time = sleep_budget;
+                }
+            }
         } while ((target_delta_time - delta_time) >
             (sleep_time / (ecs_ftime_t)2.0));
     }
@@ -261,6 +343,18 @@ void ecs_set_target_fps(
     flecs_poly_assert(world, ecs_world_t);
     ecs_check(ecs_os_has_time(), ECS_MISSING_OS_API, NULL);
 
+    /* Frame time is the reciprocal of the target, so targets outside of this
+     * range yield a frame time that is infinite (the reciprocal of a subnormal
+     * target overflows) or otherwise meaningless. The comparisons also reject
+     * NaN, and compile for any type ecs_ftime_t is defined to. In release
+     * builds this check is compiled out, and is backstopped by ecs_sleepf
+     * refusing a duration it cannot represent: the sleep returns immediately,
+     * the clock is measured as not keeping up, and frame rate limiting is
+     * skipped for the frame instead of waiting. */
+    ecs_check(ECS_EQZERO(fps) || (fps >= (ecs_ftime_t)1e-9 &&
+        fps <= (ecs_ftime_t)1e9),
+        ECS_INVALID_PARAMETER, "fps must be zero or in the range 1e-9 to 1e9");
+
     ecs_measure_frame_time(world, true);
     world->info.target_fps = fps;
 error:

src/misc.c

@@ -85,7 +85,11 @@ ecs_time_t ecs_time_sub(
 void ecs_sleepf(
     double t)
 {
-    if (t > 0) {
+    /* Only sleep for a duration that can be represented. Converting a value of
+     * 2^31 seconds or more to int is undefined behavior (out-of-range
+     * conversions produce platform-dependent garbage durations), and the
+     * comparison also rejects NaN. */
+    if (t > 0 && t <= (double)INT32_MAX) {
         int sec = (int)t;
         int nsec = (int)((t - sec) * 1000000000);
         ecs_os_sleep(sec, nsec);

test/core/project.json

@@ -2790,8 +2790,22 @@
                 "get_delta_time",
                 "get_delta_time_auto",
                 "progress_w_stalled_clock",
+                "progress_w_stalled_clock_w_target_fps",
+                "progress_w_stalled_clock_w_noop_sleep",
                 "progress_w_stalled_clock_at_zero",
                 "progress_w_coarse_clock",
+                "progress_w_coarse_clock_w_target_fps",
+                "progress_w_sleep_driven_clock",
+                "progress_w_sleep_driven_clock_w_work",
+                "progress_w_sleep_driven_clock_w_slow_frame",
+                "progress_w_clock_above_keeping_up",
+                "progress_w_clock_below_keeping_up",
+                "progress_w_dither_clock",
+                "progress_w_dither_clock_only",
+                "progress_w_stepped_clock",
+                "sleepf_w_unrepresentable_duration",
+                "set_target_fps_w_negative",
+                "set_target_fps_w_subnormal",
                 "recreate_world",
                 "recreate_world_w_component",
                 "no_threading",