dotnet / dotnet/reactive

AsyncSchedulerBase's absolute scheduling does not cope with clock drift

Open
#2,340 0 comments 0 reactions 0 assignees View on GitHub
[area] AsyncRx
Dominant language
C#
Stars
7.2k
Forks
798
PR merge metrics
No merged PRs in 30d

Description

### Background

The `IAsyncScheduler` defines a method allowing work to be scheduled at a specific time in the future:

```cs
ValueTask ScheduleAsync(Func action, DateTimeOffset dueTime);
```

This is one of three overloads the interface defines. There's this absolute time one, and also a relative time (`TimeSpan`) and an immediate one.

The `AsyncSchedulerBase` only requires derived types to implement a single, immediate method:

```cs
protected abstract ValueTask ScheduleAsyncCore(Func action, CancellationToken token);
```

It implements the two time-based versions for you with `async` methods that `await` until the due time. To enable derived types to define how time passes, they also implement this method:

```cs
protected abstract ValueTask Delay(TimeSpan dueTime, CancellationToken token);
```

This is fine for virtual time, but it is problematic for real time: it does not cope with changes to system time. If the system time as adjusted, work scheduled for an absolute time will seem to run at the wrong time. To see why, look at the implementation of the `DateTimeOffset dueTime` overload of `ScheduleAsync`:

```cs
return ScheduleAsyncCore(async ct =>
{
var dueTimeRelative = Normalize(dueTime - Now); // TODO: Support clock drift and clock changes.

await Delay(dueTimeRelative, ct); // NB: Honor SynchronizationContext to stay on scheduler.

await action(ct);
});
```

The comment shows that the original author was aware of this shortcoming. The problem is that this converts the absolute time to a `TimeSpan`, which means if the system clock changes after this was scheduled, that time change won't be taken into account.

This could cause work to be processed in the wrong order. Suppose the following happens:

* System time is 09:00:00+0
* Work scheduled for 09:01:00+0: converted to delay of 60 seconds
* One second later, the system clock is adjusted to 08:59:30+0
* Work scheduled for 09:00:50+0, converted to a delay of 80 seconds
* 59 seconds later (system time 09:00:29+0), the work scheduled for 09:01:00+0 runs
* 21 seconds later (system time 09:00:50+0) the work scheduled for 09:00:50+0 runs

So work runs out of order here. The second work item to run was scheduled to run 10 seconds earlier than the first one but actually ran 21 seconds later. To avoid this we'd need to detect clock changes (e.g. by looking at the top of the queue and seeing if its scheduled time still corresponds to the one we'd calculate if scheduling it right now).

Review what classic Rx does here - it might be that we don't solve the problem correctly there, in which case the resolution might simply be to document that absolute scheduling is just like this.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with IAsyncScheduler and AsyncSchedulerBase's absolute ScheduleAsync overload, then inspect how Now, Normalize, Delay, and ScheduleAsyncCore interact. Compare the behavior with classic Rx as requested, including clock adjustments and queue ordering. Done means either absolute scheduling responds correctly to clock changes or the limitation is clearly documented.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
backend
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.