Skip to content

Document effect of a suspended system on delay #2354

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

Closed
wants to merge 1 commit into from

Conversation

breyed
Copy link
Contributor

@breyed breyed commented Apr 24, 2019

This poor guy has to reverse engineer Task.Delay to figure out what would happen if the system were suspended after it was called. This change documents the behavior.

Someone should verify that the behavior is uniform for all platforms, and if not, document the variations.

[This poor guy](https://stackoverflow.com/q/38207026/145173) has to reverse engineer `Task.Delay` to figure out what would happen if the system were suspended after it was called. This change documents the behavior.

Someone should verify that the behavior is uniform for all platforms, and if not, document the variations.
@rpetrusha
Copy link

@stephentoub, can you comment on this PR?

@rpetrusha rpetrusha added this to the April 2019 milestone Apr 25, 2019
@stephentoub
Copy link
Member

This feels a little strange to me, e.g. Task.Delay just wraps a timer, and System.Threading.Timer doesn't have any such low-level language. The closest it gets is saying "the Timer class has the same resolution as the system clock".

@breyed
Copy link
Contributor Author

breyed commented Apr 25, 2019

The situation goes even further back than System.Threading.Timer. SetTimer is equally silent on behavior when suspended. By contrast, SetWaitableTimer discusses suspension; it needs to since it has an option to resume from standby when the timer expires.

Behavior when suspended is part of the core interface definition of a timer. Interrupt and process scheduling details that affect delays of a few milliseconds are low-level, but considerations that can impact the answer to "When will the timer expire?" by hours or days are part of the timer's primary contract.

My guess is that initially SetTimer was written and documented before Windows could suspend. When suspension was added, SetWaitableTimer came with it, but the existing docs weren't revisited. When the .NET timer came out, they followed SetTimer, since it also takes a relative time. Thus, things never progressed past the 1990s. Fast forward to today, and suspension and battery management are now first class citizens, and should be integrated into API design and documentation accordingly.

@breyed
Copy link
Contributor Author

breyed commented Apr 26, 2019

In the process of looking for a timer with expiration based on an absolute time (which is sorely needed in .NET IMHO), I ran into SetThreadpoolTimer. It is more recent than its Win32 ilk, and documents its suspend behavior well:

If the due time specified by pftDueTime is relative, the time that the system spends in sleep or hibernation does not count toward the expiration of the timer. The timer is signaled when the cumulative amount of elapsed time the system spends in the waking state equals the timer's relative due time or period. If the due time specified by pftDueTime is absolute, the time that the system spends in sleep or hibernation does count toward the expiration of the timer. If the timer expires while the system is sleeping, the timer is signaled immediately when the system wakes.

As far as I can tell, there's nothing unique about the relative time functionality of SetThreadpoolTimer that would be a reason for only its behavior to be documented.

@tdykstra
Copy link
Contributor

@stephentoub -- What do you think of the response to your review -- given that Task.Delay just wraps a timer, should this PR be closed in favor of a new one that updates System.Threading.Timer instead? Or does the added explanation still seem unnecessary in either place?

@mairaw mairaw requested a review from stephentoub October 22, 2019 06:26
@mairaw mairaw added the waiting-on-feedback Indicates PRs that are waiting for feedback from SMEs before they can be merged label Oct 22, 2019
@stephentoub
Copy link
Member

I'm fine personally with it being documented, but Task.Delay isn't the place to do it IMO. If we want to document suspension behavior, we should do it on System.Threading.Timer, and then we could say in Task.Delay's docs that it has the same behavior as Timer.
cc: @kouvel, @tarekgh in case they ahve a different opinion

@tdykstra
Copy link
Contributor

tdykstra commented Feb 3, 2020

@breyed Do you want to use this PR to implement the recommendation from @stephentoub, or should we close the PR?

@breyed
Copy link
Contributor Author

breyed commented Feb 4, 2020

@tdykstra I think you mean @stephentoub's recommendation. I agree with the recommendation, but don't have time to make the change. I don't think this PR should be just plain closed, however. Either it should be assigned to someone with bandwidth for it, or if no one is, an issue about the lack of documentation should be opened so that the work can be prioritized.

@tdykstra
Copy link
Contributor

tdykstra commented Feb 4, 2020

I think you mean stephentoub's recommendation

Yes, sorry, mistyped that name. Closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-on-feedback Indicates PRs that are waiting for feedback from SMEs before they can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants