search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.98k stars 4.66k forks source link

Timeout.InfiniteTimeSpan behaves counter-intuitively for some comparisons #37777

Open invidious9000 opened 4 years ago

invidious9000 commented 4 years ago

Description

Timeout.InfiniteTimeSpan uses an internal representation of -1ms to define a period of infinite time. The operators for <, >, >=, <= in TimeSpan compare ticks between two instances. This leads to comparisons that seem invalid when naively using InfiniteTimeSpan (negative ticks) on one side of the comparison.

Example:

var oneSecond = TimeSpan.FromSeconds(1);
bool oneSecondIsLessThanInfinity = oneSecond < Timeout.InfiniteTimeSpan;

In this scenario oneSecondIsLessThanInfinity is false when one might expect it to be true.

.NET Core 3.1

danmoseley commented 4 years ago

FWIW, same on .NET Framework.

stephentoub commented 4 years ago

I agree it's not intuitive, but there's very little that can be done about this. The meaning of the -1 value as a timeout is entirely up to how the consumer is defined and interprets it. Changing the comparison operations to special-case that value would also be very unintuitive, and a massive breaking change.

invidious9000 commented 4 years ago

TimeSpan remarks indicate that the value contained is some count of ticks, and this can be enough information to reason about the behavior, but the comparison operators seem to be a slightly more opaque implementation detail (you must look at source to see why it behaves this way).

Could this be considered a doc issue? I see that kinda-similar concerns were identified/discussed in https://github.com/dotnet/runtime/issues/30723#issuecomment-574308350

This really should be a doc issue demonstrating correct use of the existing Timeout API

Is there any value in augmenting documentation for InfiniteTimeSpan to explicitly indicate nuances around comparison or is it reasonable to expect consumers to infer these from tick remarks? Would the same suggestions advocating for IntelliCode enhancements as outlined in https://github.com/dotnet/runtime/issues/30723#issuecomment-526676911 apply?

Could it be appropriate to add a see-also for TimeSpan.MaxValue if that can sometimes be an acceptable substitute?

stephentoub commented 4 years ago

Thanks for the suggestions. Improving the docs is certainly possible! And we welcome contributions there, too. https://github.com/dotnet/dotnet-api-docs