Skip to content

Commit 575e02b

Browse files
Simonclaude
Simon
and
claude
committed
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, a stall budget of one frame period that never refills within a frame ends it. 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. Backing the budget is a ceiling on the number of times a frame may sleep, which no clock reaches: an interval that keeps up advances the measured delta by a quarter of itself while the exit condition needs only eight initial intervals of advance, and one that does not keep up is charged to the budget. Giving the clock complete freedom to answer each read with whichever value prolongs the loop, and searching that exhaustively, the worst case is 36 iterations and 4.75 frame periods of requested sleep, or 41 iterations if the clock may also run backwards. The ceiling is 128. The ecs_set_target_fps documentation states the resulting contract: a clock that observes at least a quarter of each interval reaches the target and pays the disagreement in blocking time, and one below that has the budget end its frame. 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 pin that the first reaches the target and the second stops where the budget runs out; 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 575e02b

10 files changed

Lines changed: 655 additions & 11 deletions

File tree

‎distr/flecs.c‎

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4140,6 +4140,11 @@ extern const ecs_entity_t EcsFlag;
41404140
* to an integer or fixed point type. */
41414141
#define ECS_FRAME_MIN_DELTA_TIME ((ecs_ftime_t)1e-9)
41424142

4143+
/* Ceiling on the number of times frame rate limiting sleeps within one frame.
4144+
* Reaching the target takes a few tens of intervals on any clock that keeps up,
4145+
* and a clock that does not exhausts the stall budget sooner. */
4146+
#define ECS_FRAME_MAX_SLEEP_ITERATIONS (128)
4147+
41434148
////////////////////////////////////////////////////////////////////////////////
41444149
//// Bootstrap API
41454150
////////////////////////////////////////////////////////////////////////////////
@@ -13362,7 +13367,10 @@ ecs_time_t ecs_time_sub(
1336213367
void ecs_sleepf(
1336313368
double t)
1336413369
{
13365-
if (t > 0) {
13370+
/* Refuse durations that cannot be converted to int seconds, as an
13371+
* out-of-range conversion is undefined behavior. The comparison also
13372+
* rejects NaN. */
13373+
if (t > 0 && t <= (double)INT32_MAX) {
1336613374
int sec = (int)t;
1336713375
int nsec = (int)((t - sec) * 1000000000);
1336813376
ecs_os_sleep(sec, nsec);
@@ -24373,7 +24381,21 @@ static ecs_ftime_t flecs_insert_sleep(
2437324381
sleep_time = 0;
2437424382
delta_time = (ecs_ftime_t)ecs_time_measure(&now);
2437524383
} else {
24384+
/* Sleep interval while the clock keeps up, largest delta measured so
24385+
* far, and how much was slept while stalled this frame. */
24386+
ecs_ftime_t initial_sleep_time = sleep_time;
24387+
ecs_ftime_t max_delta_time = delta_time;
24388+
ecs_ftime_t stalled_sleep = 0;
24389+
int32_t iterations = 0;
24390+
2437624391
do {
24392+
/* Ceiling on the sleeps a single frame may perform. Keeping up
24393+
* advances the measured delta by a quarter of the interval and
24394+
* stalling spends the budget below, so this is not reached. */
24395+
if (++iterations > ECS_FRAME_MAX_SLEEP_ITERATIONS) {
24396+
break;
24397+
}
24398+
2437724399
/* Only call sleep when sleep_time is not 0. On some platforms, even
2437824400
* a sleep with a timeout of 0 can cause stutter. */
2437924401
if (ECS_NEQZERO(sleep_time)) {
@@ -24382,6 +24404,46 @@ static ecs_ftime_t flecs_insert_sleep(
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: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13174,6 +13174,23 @@ 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, to within roughly a factor of four. Both may run
13179+
* on a virtualized timeline of any speed, but an application that replaces one
13180+
* of them must replace the other consistently. A clock that observes at least
13181+
* a quarter of each interval slept reaches the target frame time to within a
13182+
* sixteenth of it, blocking for as long as the disagreement costs, which stays
13183+
* under five frame periods. If
13184+
* the two disagree by more than that (for example a clock that is only stepped
13185+
* by a host in between frames, while sleeping does take time) frame rate
13186+
* limiting requests about one frame period worth of sleep and is then skipped
13187+
* for the remainder of that frame. If the clock is too coarse to observe an
13188+
* individual sleep, sleep intervals are enlarged until it can, which makes
13189+
* pacing coarser but keeps it working. A frame sleeps at most 128 times
13190+
* whatever the clock does, so it never waits for one indefinitely. An
13191+
* application that computes its target from untrusted input should range check
13192+
* it before passing it in.
13193+
*
1317713194
* @param world The world.
1317813195
* @param fps The target FPS.
1317913196
*/

‎distr/flecs_no_addons.c‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2605,6 +2605,11 @@ extern const ecs_entity_t EcsFlag;
26052605
* to an integer or fixed point type. */
26062606
#define ECS_FRAME_MIN_DELTA_TIME ((ecs_ftime_t)1e-9)
26072607

2608+
/* Ceiling on the number of times frame rate limiting sleeps within one frame.
2609+
* Reaching the target takes a few tens of intervals on any clock that keeps up,
2610+
* and a clock that does not exhausts the stall budget sooner. */
2611+
#define ECS_FRAME_MAX_SLEEP_ITERATIONS (128)
2612+
26082613
////////////////////////////////////////////////////////////////////////////////
26092614
//// Bootstrap API
26102615
////////////////////////////////////////////////////////////////////////////////
@@ -11380,7 +11385,10 @@ ecs_time_t ecs_time_sub(
1138011385
void ecs_sleepf(
1138111386
double t)
1138211387
{
11383-
if (t > 0) {
11388+
/* Refuse durations that cannot be converted to int seconds, as an
11389+
* out-of-range conversion is undefined behavior. The comparison also
11390+
* rejects NaN. */
11391+
if (t > 0 && t <= (double)INT32_MAX) {
1138411392
int sec = (int)t;
1138511393
int nsec = (int)((t - sec) * 1000000000);
1138611394
ecs_os_sleep(sec, nsec);

‎include/flecs/addons/frame.h‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,23 @@ 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, to within roughly a factor of four. Both may run
145+
* on a virtualized timeline of any speed, but an application that replaces one
146+
* of them must replace the other consistently. A clock that observes at least
147+
* a quarter of each interval slept reaches the target frame time to within a
148+
* sixteenth of it, blocking for as long as the disagreement costs, which stays
149+
* under five frame periods. If
150+
* the two disagree by more than that (for example a clock that is only stepped
151+
* by a host in between frames, while sleeping does take time) frame rate
152+
* limiting requests about one frame period worth of sleep and is then skipped
153+
* for the remainder of that frame. If the clock is too coarse to observe an
154+
* individual sleep, sleep intervals are enlarged until it can, which makes
155+
* pacing coarser but keeps it working. A frame sleeps at most 128 times
156+
* whatever the clock does, so it never waits for one indefinitely. An
157+
* application that computes its target from untrusted input should range check
158+
* it before passing it in.
159+
*
143160
* @param world The world.
144161
* @param fps The target FPS.
145162
*/

‎src/addons/frame.c‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,21 @@ 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 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+
int32_t iterations = 0;
45+
3946
do {
47+
/* Ceiling on the sleeps a single frame may perform. Keeping up
48+
* advances the measured delta by a quarter of the interval and
49+
* stalling spends the budget below, so this is not reached. */
50+
if (++iterations > ECS_FRAME_MAX_SLEEP_ITERATIONS) {
51+
break;
52+
}
53+
4054
/* Only call sleep when sleep_time is not 0. On some platforms, even
4155
* a sleep with a timeout of 0 can cause stutter. */
4256
if (ECS_NEQZERO(sleep_time)) {
@@ -45,6 +59,46 @@ static ecs_ftime_t flecs_insert_sleep(
4559

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

320+
/* Targets outside this range yield a frame time that is infinite (the
321+
* reciprocal of a subnormal target overflows) or meaningless, and the
322+
* comparison rejects NaN. In release builds ecs_sleepf backstops the
323+
* compiled-out check by refusing durations it cannot represent. */
324+
ecs_check(ECS_EQZERO(fps) || (fps >= (ecs_ftime_t)1e-9 &&
325+
fps <= (ecs_ftime_t)1e9),
326+
ECS_INVALID_PARAMETER, "fps must be zero or in the range 1e-9 to 1e9");
327+
266328
ecs_measure_frame_time(world, true);
267329
world->info.target_fps = fps;
268330
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);

‎src/private_api.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@ extern const ecs_entity_t EcsFlag;
6363
* to an integer or fixed point type. */
6464
#define ECS_FRAME_MIN_DELTA_TIME ((ecs_ftime_t)1e-9)
6565

66+
/* Ceiling on the number of times frame rate limiting sleeps within one frame.
67+
* Reaching the target takes a few tens of intervals on any clock that keeps up,
68+
* and a clock that does not exhausts the stall budget sooner. */
69+
#define ECS_FRAME_MAX_SLEEP_ITERATIONS (128)
70+
6671
////////////////////////////////////////////////////////////////////////////////
6772
//// Bootstrap API
6873
////////////////////////////////////////////////////////////////////////////////

‎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)