Skip to content

[FuzzingEventEngine] fix TickForDuration() bug and add test#34633

Merged
ctiller merged 3 commits intogrpc:masterfrom
markdroth:fuzzing_ee_tick_for_duration_fix
Oct 11, 2023
Merged

[FuzzingEventEngine] fix TickForDuration() bug and add test#34633
ctiller merged 3 commits intogrpc:masterfrom
markdroth:fuzzing_ee_tick_for_duration_fix

Conversation

@markdroth
Copy link
Copy Markdown
Member

The TickForDuration() method was using grpc_core::Timestamp::Now() to get the current time, but that was not in sync with the now_ value inside the Fuzzing EE itself, with the result that after two subsequent 250ms increments, timers were not being properly fired. I've added a test that demonstrates this failure without the fix.

@drfloob
Copy link
Copy Markdown
Member

drfloob commented Oct 10, 2023

Excellent find.

@markdroth markdroth changed the title [FuzzingEventEngine] fix TickForDuration() bug and add test [FuzzingEventEngine] fix TickForDuration() bug and add test Oct 11, 2023
@markdroth markdroth changed the title [FuzzingEventEngine] fix TickForDuration() bug and add test [FuzzingEventEngine] fix TickForDuration() bug and add test Oct 11, 2023
@markdroth markdroth added release notes: no Indicates if PR should not be in release notes and removed release notes: no Indicates if PR should not be in release notes labels Oct 11, 2023
@ctiller ctiller merged commit be1cf35 into grpc:master Oct 11, 2023
@markdroth markdroth deleted the fuzzing_ee_tick_for_duration_fix branch October 11, 2023 15:26
@copybara-service copybara-service bot added the imported Specifies if the PR has been imported to the internal repository label Oct 12, 2023
ctiller pushed a commit to ctiller/grpc that referenced this pull request Oct 20, 2023
The `TickForDuration()` method was using `grpc_core::Timestamp::Now()`
to get the current time, but that was not in sync with the `now_` value
inside the Fuzzing EE itself, with the result that after two subsequent
250ms increments, timers were not being properly fired. I've added a
test that demonstrates this failure without the fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bloat/none imported Specifies if the PR has been imported to the internal repository lang/core per-call-memory/neutral per-channel-memory/neutral release notes: no Indicates if PR should not be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants