Skip to content

Commit 74b1c4e

Browse files
Simonclaude
andcommitted
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>
1 parent c7c3d0e commit 74b1c4e

9 files changed

Lines changed: 631 additions & 11 deletions

File tree

distr/flecs.c

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13362,7 +13362,10 @@ ecs_time_t ecs_time_sub(
1336213362
void ecs_sleepf(
1336313363
double t)
1336413364
{
13365-
if (t > 0) {
13365+
/* Refuse durations that cannot be converted to int seconds, as an
13366+
* out-of-range conversion is undefined behavior. The comparison also
13367+
* rejects NaN. */
13368+
if (t > 0 && t <= (double)INT32_MAX) {
1336613369
int sec = (int)t;
1336713370
int nsec = (int)((t - sec) * 1000000000);
1336813371
ecs_os_sleep(sec, nsec);
@@ -24373,15 +24376,74 @@ static ecs_ftime_t flecs_insert_sleep(
2437324376
sleep_time = 0;
2437424377
delta_time = (ecs_ftime_t)ecs_time_measure(&now);
2437524378
} else {
24379+
/* Sleep interval while the clock keeps up, largest delta measured so
24380+
* far, and how much was slept while stalled / in total this frame. */
24381+
ecs_ftime_t initial_sleep_time = sleep_time;
24382+
ecs_ftime_t max_delta_time = delta_time;
24383+
ecs_ftime_t stalled_sleep = 0;
24384+
ecs_ftime_t total_sleep = 0;
24385+
2437624386
do {
24387+
/* Unconditional bound on the total sleep requested for a single
24388+
* frame, independent of the stall classification below. */
24389+
ecs_ftime_t sleep_cap =
24390+
(target_delta_time * (ecs_ftime_t)2.0) - total_sleep;
24391+
if (sleep_cap <= 0) {
24392+
break;
24393+
}
24394+
if (sleep_time > sleep_cap) {
24395+
sleep_time = sleep_cap;
24396+
}
24397+
2437724398
/* Only call sleep when sleep_time is not 0. On some platforms, even
2437824399
* a sleep with a timeout of 0 can cause stutter. */
2437924400
if (ECS_NEQZERO(sleep_time)) {
2438024401
ecs_sleepf((double)sleep_time);
24402+
total_sleep += sleep_time;
2438124403
}
2438224404

2438324405
now = start;
2438424406
delta_time = (ecs_ftime_t)ecs_time_measure(&now);
24407+
24408+
/* The clock keeps up if it advanced by a meaningful fraction of
24409+
* the interval just slept. Any advance at all would qualify a
24410+
* clock that ticks a nanosecond per read. */
24411+
if ((delta_time - max_delta_time) >=
24412+
(sleep_time / (ecs_ftime_t)4.0))
24413+
{
24414+
max_delta_time = delta_time;
24415+
24416+
/* Restore the interval, as the exit condition is expressed in
24417+
* terms of it and would overshoot the target while escalated. */
24418+
sleep_time = initial_sleep_time;
24419+
} else {
24420+
/* The clock is too coarse to measure an interval this small,
24421+
* or not running at all. Accumulate the requested interval,
24422+
* not measured time, so neither the clock nor a replaced
24423+
* sleep function needs to make progress for the frame to end. */
24424+
stalled_sleep += sleep_time;
24425+
24426+
/* At most one frame's worth of sleep on a clock that is not
24427+
* keeping up. Never refills within the frame; a budget that
24428+
* refills lets a clock that advances just enough to be
24429+
* counted replay the escalation chain indefinitely. */
24430+
ecs_ftime_t sleep_budget = target_delta_time - stalled_sleep;
24431+
if (sleep_budget <= 0) {
24432+
break;
24433+
}
24434+
24435+
if (ECS_EQZERO(sleep_time)) {
24436+
/* Doubling zero stays zero, so seed the escalation with a
24437+
* nonzero interval. */
24438+
sleep_time = target_delta_time / (ecs_ftime_t)8.0;
24439+
} else {
24440+
sleep_time *= (ecs_ftime_t)2.0;
24441+
}
24442+
24443+
if (sleep_time > sleep_budget) {
24444+
sleep_time = sleep_budget;
24445+
}
24446+
}
2438524447
} while ((target_delta_time - delta_time) >
2438624448
(sleep_time / (ecs_ftime_t)2.0));
2438724449
}
@@ -24600,6 +24662,14 @@ void ecs_set_target_fps(
2460024662
flecs_poly_assert(world, ecs_world_t);
2460124663
ecs_check(ecs_os_has_time(), ECS_MISSING_OS_API, NULL);
2460224664

24665+
/* Targets outside this range yield a frame time that is infinite (the
24666+
* reciprocal of a subnormal target overflows) or meaningless, and the
24667+
* comparison rejects NaN. In release builds ecs_sleepf backstops the
24668+
* compiled-out check by refusing durations it cannot represent. */
24669+
ecs_check(ECS_EQZERO(fps) || (fps >= (ecs_ftime_t)1e-9 &&
24670+
fps <= (ecs_ftime_t)1e9),
24671+
ECS_INVALID_PARAMETER, "fps must be zero or in the range 1e-9 to 1e9");
24672+
2460324673
ecs_measure_frame_time(world, true);
2460424674
world->info.target_fps = fps;
2460524675
error:

distr/flecs.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13174,6 +13174,22 @@ void ecs_measure_system_time(
1317413174
* Note that ecs_progress() only sleeps if there is time left in the frame. Both
1317513175
* time spent in Flecs and time spent outside of Flecs are taken into account.
1317613176
*
13177+
* Frame rate limiting requires the OS API get_time and sleep functions to agree
13178+
* about the passage of time. Both may run on a virtualized timeline of any
13179+
* speed, but an application that replaces one of them must replace the other
13180+
* consistently. The total sleep requested for a single frame is capped at two
13181+
* frame periods, so a frame never waits for the clock indefinitely. The cap is
13182+
* measured in requested sleep, so a clock that observes less than about half
13183+
* of each sleep ends the frame short of the target, in proportion to the
13184+
* disagreement. If clock and sleep disagree by more than roughly a factor of
13185+
* four (for example a clock that is only stepped by a host in between frames,
13186+
* while sleeping does take time) frame rate limiting requests about one frame
13187+
* period worth of sleep and is then skipped for the remainder of that frame.
13188+
* If the clock is too coarse to observe an individual sleep, sleep intervals
13189+
* are enlarged until it can, which makes pacing coarser but keeps it working.
13190+
* An application that computes its target from untrusted input should range
13191+
* check it before passing it in.
13192+
*
1317713193
* @param world The world.
1317813194
* @param fps The target FPS.
1317913195
*/

distr/flecs_no_addons.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11380,7 +11380,10 @@ ecs_time_t ecs_time_sub(
1138011380
void ecs_sleepf(
1138111381
double t)
1138211382
{
11383-
if (t > 0) {
11383+
/* Refuse durations that cannot be converted to int seconds, as an
11384+
* out-of-range conversion is undefined behavior. The comparison also
11385+
* rejects NaN. */
11386+
if (t > 0 && t <= (double)INT32_MAX) {
1138411387
int sec = (int)t;
1138511388
int nsec = (int)((t - sec) * 1000000000);
1138611389
ecs_os_sleep(sec, nsec);

include/flecs/addons/frame.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,22 @@ void ecs_measure_system_time(
140140
* Note that ecs_progress() only sleeps if there is time left in the frame. Both
141141
* time spent in Flecs and time spent outside of Flecs are taken into account.
142142
*
143+
* Frame rate limiting requires the OS API get_time and sleep functions to agree
144+
* about the passage of time. Both may run on a virtualized timeline of any
145+
* speed, but an application that replaces one of them must replace the other
146+
* consistently. The total sleep requested for a single frame is capped at two
147+
* frame periods, so a frame never waits for the clock indefinitely. The cap is
148+
* measured in requested sleep, so a clock that observes less than about half
149+
* of each sleep ends the frame short of the target, in proportion to the
150+
* disagreement. If clock and sleep disagree by more than roughly a factor of
151+
* four (for example a clock that is only stepped by a host in between frames,
152+
* while sleeping does take time) frame rate limiting requests about one frame
153+
* period worth of sleep and is then skipped for the remainder of that frame.
154+
* If the clock is too coarse to observe an individual sleep, sleep intervals
155+
* are enlarged until it can, which makes pacing coarser but keeps it working.
156+
* An application that computes its target from untrusted input should range
157+
* check it before passing it in.
158+
*
143159
* @param world The world.
144160
* @param fps The target FPS.
145161
*/

src/addons/frame.c

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,74 @@ static ecs_ftime_t flecs_insert_sleep(
3636
sleep_time = 0;
3737
delta_time = (ecs_ftime_t)ecs_time_measure(&now);
3838
} else {
39+
/* Sleep interval while the clock keeps up, largest delta measured so
40+
* far, and how much was slept while stalled / in total this frame. */
41+
ecs_ftime_t initial_sleep_time = sleep_time;
42+
ecs_ftime_t max_delta_time = delta_time;
43+
ecs_ftime_t stalled_sleep = 0;
44+
ecs_ftime_t total_sleep = 0;
45+
3946
do {
47+
/* Unconditional bound on the total sleep requested for a single
48+
* frame, independent of the stall classification below. */
49+
ecs_ftime_t sleep_cap =
50+
(target_delta_time * (ecs_ftime_t)2.0) - total_sleep;
51+
if (sleep_cap <= 0) {
52+
break;
53+
}
54+
if (sleep_time > sleep_cap) {
55+
sleep_time = sleep_cap;
56+
}
57+
4058
/* Only call sleep when sleep_time is not 0. On some platforms, even
4159
* a sleep with a timeout of 0 can cause stutter. */
4260
if (ECS_NEQZERO(sleep_time)) {
4361
ecs_sleepf((double)sleep_time);
62+
total_sleep += sleep_time;
4463
}
4564

4665
now = start;
4766
delta_time = (ecs_ftime_t)ecs_time_measure(&now);
67+
68+
/* The clock keeps up if it advanced by a meaningful fraction of
69+
* the interval just slept. Any advance at all would qualify a
70+
* clock that ticks a nanosecond per read. */
71+
if ((delta_time - max_delta_time) >=
72+
(sleep_time / (ecs_ftime_t)4.0))
73+
{
74+
max_delta_time = delta_time;
75+
76+
/* Restore the interval, as the exit condition is expressed in
77+
* terms of it and would overshoot the target while escalated. */
78+
sleep_time = initial_sleep_time;
79+
} else {
80+
/* The clock is too coarse to measure an interval this small,
81+
* or not running at all. Accumulate the requested interval,
82+
* not measured time, so neither the clock nor a replaced
83+
* sleep function needs to make progress for the frame to end. */
84+
stalled_sleep += sleep_time;
85+
86+
/* At most one frame's worth of sleep on a clock that is not
87+
* keeping up. Never refills within the frame; a budget that
88+
* refills lets a clock that advances just enough to be
89+
* counted replay the escalation chain indefinitely. */
90+
ecs_ftime_t sleep_budget = target_delta_time - stalled_sleep;
91+
if (sleep_budget <= 0) {
92+
break;
93+
}
94+
95+
if (ECS_EQZERO(sleep_time)) {
96+
/* Doubling zero stays zero, so seed the escalation with a
97+
* nonzero interval. */
98+
sleep_time = target_delta_time / (ecs_ftime_t)8.0;
99+
} else {
100+
sleep_time *= (ecs_ftime_t)2.0;
101+
}
102+
103+
if (sleep_time > sleep_budget) {
104+
sleep_time = sleep_budget;
105+
}
106+
}
48107
} while ((target_delta_time - delta_time) >
49108
(sleep_time / (ecs_ftime_t)2.0));
50109
}
@@ -263,6 +322,14 @@ void ecs_set_target_fps(
263322
flecs_poly_assert(world, ecs_world_t);
264323
ecs_check(ecs_os_has_time(), ECS_MISSING_OS_API, NULL);
265324

325+
/* Targets outside this range yield a frame time that is infinite (the
326+
* reciprocal of a subnormal target overflows) or meaningless, and the
327+
* comparison rejects NaN. In release builds ecs_sleepf backstops the
328+
* compiled-out check by refusing durations it cannot represent. */
329+
ecs_check(ECS_EQZERO(fps) || (fps >= (ecs_ftime_t)1e-9 &&
330+
fps <= (ecs_ftime_t)1e9),
331+
ECS_INVALID_PARAMETER, "fps must be zero or in the range 1e-9 to 1e9");
332+
266333
ecs_measure_frame_time(world, true);
267334
world->info.target_fps = fps;
268335
error:

src/misc.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,10 @@ ecs_time_t ecs_time_sub(
8585
void ecs_sleepf(
8686
double t)
8787
{
88-
if (t > 0) {
88+
/* Refuse durations that cannot be converted to int seconds, as an
89+
* out-of-range conversion is undefined behavior. The comparison also
90+
* rejects NaN. */
91+
if (t > 0 && t <= (double)INT32_MAX) {
8992
int sec = (int)t;
9093
int nsec = (int)((t - sec) * 1000000000);
9194
ecs_os_sleep(sec, nsec);

test/core/project.json

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2790,9 +2790,23 @@
27902790
"get_delta_time",
27912791
"get_delta_time_auto",
27922792
"progress_w_stalled_clock",
2793+
"progress_w_stalled_clock_w_target_fps",
2794+
"progress_w_stalled_clock_w_noop_sleep",
27932795
"progress_w_stalled_clock_at_zero",
27942796
"progress_w_stalled_clock_warns_once",
27952797
"progress_w_coarse_clock",
2798+
"progress_w_coarse_clock_w_target_fps",
2799+
"progress_w_sleep_driven_clock",
2800+
"progress_w_sleep_driven_clock_w_work",
2801+
"progress_w_sleep_driven_clock_w_slow_frame",
2802+
"progress_w_clock_above_keeping_up",
2803+
"progress_w_clock_below_keeping_up",
2804+
"progress_w_dither_clock",
2805+
"progress_w_dither_clock_only",
2806+
"progress_w_stepped_clock",
2807+
"sleepf_w_unrepresentable_duration",
2808+
"set_target_fps_w_negative",
2809+
"set_target_fps_w_subnormal",
27962810
"recreate_world",
27972811
"recreate_world_w_component",
27982812
"no_threading",

0 commit comments

Comments
 (0)