Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG]: IOScheduler yield_util execution semantics are incorrect #532

Open
2 tasks done
willkill07 opened this issue Jan 30, 2025 · 0 comments
Open
2 tasks done

[BUG]: IOScheduler yield_util execution semantics are incorrect #532

willkill07 opened this issue Jan 30, 2025 · 0 comments
Assignees
Labels
bug Something isn't working

Comments

@willkill07
Copy link
Contributor

Version

25.02

Which installation method(s) does this occur on?

Source

Describe the bug.

The current test wasn't asserting anything, so I added naive assumptions that resulted in consistent failure.

Looking at the implementation, there is a duration_cast<std::chrono::milliseconds>() which produces information loss and alters the expected behavior.

Minimum reproducible example

TEST_F(TestCoroIoScheduler, YieldUntil)
{
    auto scheduler = coroutines::IoScheduler::get_instance();

    coroutines::clock_t::time_point target_time{};

    auto task = [scheduler, &target_time]() -> coroutines::Task<> {
        target_time = coroutines::clock_t::now() + 10ms;
        co_await scheduler->yield_until(target_time);
    };
    
    coroutines::sync_wait(task());

    auto current_time = coroutines::clock_t::now();

    ASSERT_GE(current_time, target_time);
}

Relevant log output

Full env printout

Other/Misc.

No response

Code of Conduct

  • I agree to follow MRC's Code of Conduct
  • I have searched the open bugs and have found no duplicates for this bug report
@willkill07 willkill07 added the bug Something isn't working label Jan 30, 2025
@willkill07 willkill07 self-assigned this Jan 30, 2025
@willkill07 willkill07 changed the title [BUG]: IOScheduler wait_until is buggy [BUG]: IOScheduler yield_util execution semantics are incorrect Jan 30, 2025
rapids-bot bot pushed a commit that referenced this issue Jan 30, 2025
This PR simplifies the implementation of `yield_for` and `yield_until` while also eliminating a bug in `yield_for` which can cause it to prematurely notify.

Closes #532

Authors:
  - Will Killian (https://github.com/willkill07)

Approvers:
  - David Gardner (https://github.com/dagardner-nv)

URL: #533
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant