Use TimeSpan everywhere we use an int for seconds, milliseconds, and timeouts

[briangru]

In a large software project, it's easy to mess up units for time. IE, is that Int32 seconds or milliseconds? Or maybe minutes? I've been fixing my team's source code to use the unit-agnostic TimeSpan class wherever possible. However the .NET Framework is not complete in its adoption of TimeSpan. Specifically, we don't have a Process.WaitForExit overload that takes a TimeSpan, only an Int32 for the timeout.

I suggest someone look through all .NET Framework API's for Int32 parameters containing "second", "millisecond", "ms", "timeout" (and perhaps "time") and see if there is a parallel TimeSpan-based overload. If not, please fix that.

API Proposal

(Proposed by @reflectronic)

namespace System {
    public static class GC {
+       public static GCNotificationStatus WaitForFullGCApproach(TimeSpan timeout);
+       public static GCNotificationStatus WaitForFullGCComplete(TimeSpan timeout);
    }
}

namespace System.ComponentModel.DataAnnotations {
    public class RegularExpressionAttribute {
!       This one might not end up being that useful, since people generally don't ever manipulate an instance of this attribute.
+       public TimeSpan MatchTimeout { get; }
    }
}

namespace System.Diagnostics {
    public class Process {
+       public bool WaitForExit(TimeSpan timeout);
+       public bool WaitForInputIdle(TimeSpan timeout);
    }
}

namespace System.IO {
    public class FileSystemWatcher {
+       public WaitForChangedResult WaitForChanged(WatcherChangeTypes changeType, TimeSpan timeout);
    }

    public sealed class NamedPipeClientStream : PipeStream {
+       public void Connect(TimeSpan timeout);
+       public Task ConnectAsync(TimeSpan timeout, CancellationToken cancellationToken = default);
    }
}

namespace System.Net.NetworkInformation {
    public class Ping {
+       public PingReply Send(IPAddress address, TimeSpan timeout, byte[]? buffer = null, PingOptions? options = null);
+       public PingReply Send(string hostNameOrAddress, TimeSpan timeout, byte[]? buffer = null, PingOptions? options = null);

!       Skipped EAP based methods; if they are desired, they can be added back in

!       I added CancellationToken because it is probably worth it. If you don't want it you can remove it.
+       public Task<PingReply> SendPingAsync(IPAddress address, TimeSpan timeout, byte[]? buffer = null, PingOptions? options = null, CancellationToken cancellationToken = default);
+       public Task<PingReply> SendPingAsync(string hostNameOrAddress, TimeSpan timeout, byte[]? buffer = null, PingOptions? options = null, CancellationToken cancellationToken = default);
    }
}

namespace System.Net.Sockets {
    public class NetworkStream : Stream {
+       public void Close(TimeSpan timeout);
    }

    public class Socket {
+       public bool Poll(TimeSpan timeout, SelectMode mode);
+       public static void Select(IList checkRead, IList checkWrite, IList checkError, TimeSpan timeout);
    }
}

namespace System.ServiceProcess {
    public class ServiceBase {
+       public void RequestAdditionalTime(TimeSpan time);
    }
}

namespace System.Threading.Tasks {
    public class Task {
+       public bool Wait(TimeSpan timeout, CancellationToken cancellationToken); 
    } 
} 

namespace System.Timers {
    public class Timer {
+       public Timer(TimeSpan interval);
    }
}

APIs that I don't know how to make better:

namespace System.IO {
    public abstract class Stream {
+       public TimeSpan ReadTimeoutTimeSpan { get; set; }
+       public TimeSpan WriteTimeoutTimeSpan { get; set; }
    }
}

namespace System.IO.Ports {
    public class SerialPort {
+       public TimeSpan ReadTimeoutTimeSpan { get; set; }
+       public TimeSpan WriteTimeoutTimeSpan { get; set; }
    }
}

namespace System.Media {
    public class SoundPlayer {
+       public TimeSpan LoadTimeoutTimeSpan { get; set; } 
    }
}

namespace System.Net.NetworkInformation {
+   public static class NetworkInformationTimeSpanExtensions {
+       public static TimeSpan GetPacketReassemblyTimeout(this IPGlobalStatistics statistics);
+       public static TimeSpan GetMaximumTransmissionTimeout(this TcpStatistics statistics);
+       public static TimeSpan GetMinimumTransmissionTimeout(this TcpStatistics statistics);
+   }
}

namespace System.Net.Sockets {
    public class Socket {
+       public TimeSpan ReceiveTimeoutTimeSpan { get; set; }
+       public TimeSpan ReceiveTimeoutTimeSpan { get; set; }
    }

    public class TcpClient {
+       public TimeSpan ReceiveTimeoutTimeSpan { get; set; }
+       public TimeSpan SendTimeoutTimeSpan { get; set; }
    }
}

namespace System.Timers {
    public class Timer {
+       public TimeSpan IntervalTimeSpan { get; set; }
    }
}

APIs which use seconds:

These suffer from the same problem as the previous group, but also only deal with seconds, meaning that most TimeSpan values would need to be rounded out.

namespace System.Data {
!   This would need to be a DIM.
    public interface IDbCommand {
+       TimeSpan CommandTimeoutTimeSpan { get; set; }
    }

+   public static class DataTimeSpanExtensions {
+       public static TimeSpan GetConnectionTimeout(this IDbConnection connection);
+   }

}

namespace System.Data.Sql {
    public sealed class SqlNotificationRequest {
+       public TimeSpan TimeoutTimeSpan { get; set; }
    }
}

namespace System.Data.SqlClient {
    public sealed class SqlBulkCopy {
+       public TimeSpan BulkCopyTimeoutTimeSpan { get; set; }
    }

    public sealed class SqlConnectionStringBuilder {
+       public TimeSpan ConnectTimeoutTimeSpan { get; set; }
+       public TimeSpan LoadBalanceTimeoutTimeSpan { get; set; }
    }

    public sealed class SqlDependency {
+       public SqlDependency(SqlCommand command, string options, TimeSpan timeout);
    }
}

namespace System.Net.Sockets {
    public class LingerOption {
+       public LingerOption(bool enable, TimeSpan time);
+       public TimeSpan LingerTimeSpan { get; set; }
    }

    public class Socket {
+       public void Close(TimeSpan timeout);
    }
}

[Clockwork-Muse]

I personally am against doing this right now.
Not because I think the idea has no merit (on the contrary, I'm all for using a specific type that means "duration without units"), but because I'd rather do this with a type from a rebuilt date/time library (and hit all date/time related entry points, which would be a larger undertaking).

[ellismg]

We think this is a good idea, even using the current type. What would be helpful is if someone could look at all the types we are going to expose (see https://github.com/dotnet/corefx-progress for a complete list) and produce a speclet of the APIs we'd need to add. Then we can take it into API review and start doing the work.

[aevitas]

@ellismg The request is a compiled list of all API members included in src-full that include an indication of time in minutes, seconds, milliseconds?

[sharwell]

I am very much a fan of this, and believe TimeSpan is the correct type to use.

[mattjohnsonpint]

I'm also a fan of this, as long as it is only implemented in places where time is used as a duration. (TimeSpan should not be used when referring to a time-of-day).

[terrajobst]

There is a lot of trickery due to .NET Framework compat and how we type forward contracts.

@briangru, it would be helpful if you could distill the list of APIs that we'd need to add. Then we can break it down more.

[karelz]

We need list of APIs that should be added. Then we need to review them.

[Gnbrkm41]

Time to shine some lights on this issue? :^)

System.GC.WaitForFullGCApproach(int millisecondsTimeout);
System.GC.WaitForFullGCComplete(int millisecondsTimeout);

System.Net.Sockets.Socket.Close(int timeout);
System.Net.Sockets.Socket.Poll(int microSeconds, SelectMode mode)
System.Net.Sockets.Socket.Select(IList checkRead, IList checkWrite, IList checkError, int microSeconds)
System.Net.Sockets.NetworkStream.Close(int timeout)

System.Net.Sockets.LingerOption.ctor(bool enable, int seconds)
System.Net.Sockets.LingerOption.LingerTime // int, in seconds
System.Security.Cryptography.Pkcs.Rfc3161TimestampTokenInfo.ctor(
            Oid policyId,
            Oid hashAlgorithmId,
            ReadOnlyMemory<byte> messageHash,
            ReadOnlyMemory<byte> serialNumber,
            DateTimeOffset timestamp,
            long? accuracyInMicroseconds = null,
            bool isOrdering = false,
            ReadOnlyMemory<byte>? nonce = null,
            ReadOnlyMemory<byte>? tsaName = null,
            X509ExtensionCollection extensions = null)

Perhaps could automate the searching by using Roslyn to look for method / constructor parameters and property name containing "time", "second" etc. (In fact, I tried it but failed miserably)

[Gnbrkm41]

Is there any plans regarding this issue? seems abandoned considering that it's been more than two years since the last update.

/cc @joperezr

[karelz]

@Gnbrkm41 it is in Future milestone, we do not close valid issues when we do not have time to look at them. It may be opened for longer time than "just" 2 years.
The APIs which are new in 3.0 (e.g. LingerOption) should be changed before we ship 3.0 ... having a separate issue for that would be good.
For the rest, we can add overloads in 3.0.

Are you interested in picking it up?

[Gnbrkm41]

@karelz Ah, right. I see.

If you're talking about the LingerOption, sure, I can open an issue regarding that problem.

[Gnbrkm41]

@karelz looking at the .NET API catalog, it appears that LingerOption is present since .NET Framework 1.0. Are you sure it's new in Core 3.0?

[karelz]

@Gnbrkm41 not sure anymore. I likely mixed it up with another Linger feature.

[stephentoub]

I likely mixed it up with another Linger feature.

Maybe you meant keep alive?

[karelz]

Yep, that's the one ... KeepAlive. Sorry for confusion.

[reflectronic]

Okay, I have semi-automatically generated a list of types (and the specific members) that don't use a TimeSpan where it would be appropriate:

Offending Types

namespace System {
    public static class GC {
        public static GCNotificationStatus WaitForFullGCApproach(int millisecondsTimeout);
        public static GCNotificationStatus WaitForFullGCComplete(int millisecondsTimeout);
    }
}

namespace System.ComponentModel.DataAnnotations {
    public class RegularExpressionAttribute {
        public int MatchTimeoutInMilliseconds { get; set; }
    }
}

namespace System.Data {
    public interface IDbCommand {
        int CommandTimeout { get; set; }
    }

    public interface IDbConnection {
        int ConnectionTimeout { get; }
    }

}

namespace System.Data.Sql {
    public sealed class SqlNotificationRequest {
        public int Timeout { get; set; }
    }
}

namespace System.Data.SqlClient {
    public sealed class SqlBulkCopy {
        public int BulkCopyTimeout { get; set; }
    }

    public sealed class SqlConnectionStringBuilder {
        public int ConnectTimeout { get; set; }
        public int LoadBalanceTimeout { get; set; }
    }

    public sealed class SqlDependency {
        public SqlDependency(SqlCommand command, string options, int timeout);
    }
}

namespace System.Diagnostics {
    public class Process {
        public bool WaitForExit(int milliseconds);
        public bool WaitForInputIdle(int milliseconds);
    }
}

namespace System.IO {
    public class FileSystemWatcher {
        public WaitForChangedResult WaitForChanged(WatcherChangeTypes changeType, int timeout);
    }

    public abstract class Stream {
        public virtual int ReadTimeout { get; set; }
        public virtual int WriteTimeout { get; set; }
    }
}

namespace System.IO.Pipes {
    public sealed class NamedPipeClientStream : PipeStream {
        public void Connect(int timeout);
        public Task ConnectAsync(int timeout);
        public Task ConnectAsync(int timeout, CancellationToken cancellationToken);
    }
}

namespace System.IO.Ports {
    public class SerialPort {
        public int ReadTimeout { get; set; }
        public int WriteTimeout { get; set; }
    }
}

namespace System.Media {
    public class SoundPlayer {
        public int LoadTimeout { get; set; }
    }
}

namespace System.Net {
    public sealed class FtpWebRequest : WebRequest {
        public int ReadWriteTimeout { get; set; }
    }

    public sealed class HttpWebRequest : WebRequest {
        public int ContinueTimeout { get; set; }
        public int ReadWriteTimeout { get; set; }
    }

    public class ServicePoint {
        public int ConnectionLeaseTimeout { get; set; }
    }

    public class ServicePointManager {
        public static int DnsRefreshTimeout { get; set; }
    }

    public abstract class WebRequest {
        public virtual int Timeout { get; set; }
    }
}

namespace System.Net.Mail {
    public class SmtpClient {
        public int Timeout { get; set; }
    }
}

namespace System.Net.NetworkInformation {
    public abstract class IPGlobalStatistics {
        public abstract long PacketReassemblyTimeout { get; }
    }

    public class Ping {
        public PingReply Send(IPAddress address, int timeout);
        public PingReply Send(string hostNameOrAddress, int timeout);
        public PingReply Send(IPAddress address, int timeout, byte[] buffer);
        public PingReply Send(string hostNameOrAddress, int timeout, byte[] buffer);
        public PingReply Send(IPAddress address, int timeout, byte[] buffer, PingOptions options);
        public PingReply Send(string hostNameOrAddress, int timeout, byte[] buffer, PingOptions options);
        public void SendAsync(IPAddress address, int timeout, object userToken);
        public void SendAsync(string hostNameOrAddress, int timeout, object userToken);
        public void SendAsync(IPAddress address, int timeout, byte[] buffer, object userToken);
        public void SendAsync(string hostNameOrAddress, int timeout, byte[] buffer, object userToken);
        public void SendAsync(IPAddress address, int timeout, byte[] buffer, PingOptions options, object userToken);
        public void SendAsync(string hostNameOrAddress, int timeout, byte[] buffer, PingOptions options, object userToken);
        public Task<PingReply> SendPingAsync(IPAddress address, int timeout);
        public Task<PingReply> SendPingAsync(string hostNameOrAddress, int timeout);
        public Task<PingReply> SendPingAsync(IPAddress address, int timeout, byte[] buffer);
        public Task<PingReply> SendPingAsync(string hostNameOrAddress, int timeout, byte[] buffer);
        public Task<PingReply> SendPingAsync(IPAddress address, int timeout, byte[] buffer, PingOptions options);
        public Task<PingReply> SendPingAsync(string hostNameOrAddress, int timeout, byte[] buffer, PingOptions options);
    }

    public abstract class TcpStatistics {
        public abstract long MaximumTransmissionTimeout { get; }
        public abstract long MinimumTransmissionTimeout { get; }
    }
}

namespace System.Net.Sockets {
    public class LingerOption {
        public LingerOption(bool enable, int seconds);
        public int LingerTime { get; set; }
    }

    public class NetworkStream : Stream {
        public void Close(int timeout);
    }

    public class Socket {
        public void Close(int timeout);
        public bool Poll(int microSeconds, SelectMode mode);
        public int ReceiveTimeout { get; set; }
        public int SendTimeout { get; set; }
        public static void Select(IList checkRead, IList checkWrite, IList checkError, int microSeconds);
    }

    public class TcpClient {
        public int ReceiveTimeout { get; set; }
        public int SendTimeout { get; set; }
    }
}

namespace System.Security.Cryptography.Pkcs {
    public sealed class Rfc3161TimestampTokenInfo {
        public Rfc3161TimestampTokenInfo(Oid policyId, Oid hashAlgorithmId, ReadOnlyMemory<byte> messageHash, ReadOnlyMemory<byte> serialNumber, DateTimeOffset timestamp, long? accuracyInMicroseconds, bool isOrdering, ReadOnlyMemory<byte>? nonce, ReadOnlyMemory<byte>? timestampAuthorityName, X509ExtensionCollection extensions)
        public long? AccuracyInMicroseconds { get; }
    }
}

namespace System.ServiceProcess {
    public class ServiceBase {
        public void RequestAdditionalTime(int milliseconds);
    }
}

namespace System.Threading {
    public class SynchronizationContext {
        public virtual int Wait(IntPtr[] waitHandles, bool waitAll, int millisecondsTimeout);
        protected static int WaitHelper(IntPtr[] waitHandles, bool waitAll, int millisecondsTimeout);
    }
}

namespace System.Timers {
    public class Timer {
        public Timer(double interval);
        public double Interval { get; set; }
    }
}

There isn't a good, uniform way to make all of these members better. I've split this proposal into 3 parts: APIs which I think are fine, APIs which I can't figure out how to make better, and APIs that deal in seconds (which may not be good to add such APIs to).

APIs I'm fine with:

namespace System {
    public static class GC {
+       public static GCNotificationStatus WaitForFullGCApproach(TimeSpan timeout);
+       public static GCNotificationStatus WaitForFullGCComplete(TimeSpan timeout);
    }
}

namespace System.ComponentModel.DataAnnotations {
    public class RegularExpressionAttribute {
!       This one might not end up being that useful, since people generally don't ever manipulate an instance of this attribute.
+       public TimeSpan MatchTimeout { get; }
    }
}

namespace System.Diagnostics {
    public class Process {
+       public bool WaitForExit(TimeSpan timeout);
+       public bool WaitForInputIdle(TimeSpan timeout);
    }
}

namespace System.IO {
    public class FileSystemWatcher {
+       public WaitForChangedResult WaitForChanged(WatcherChangeTypes changeType, TimeSpan timeout);
    }

    public sealed class NamedPipeClientStream : PipeStream {
+       public void Connect(TimeSpan timeout);
+       public Task ConnectAsync(TimeSpan timeout, CancellationToken cancellationToken = default);
    }
}

namespace System.Net.NetworkInformation {
    public class Ping {
+       public PingReply Send(IPAddress address, TimeSpan timeout, byte[]? buffer = null, PingOptions? options = null);
+       public PingReply Send(string hostNameOrAddress, TimeSpan timeout, byte[]? buffer = null, PingOptions? options = null);

!       Skipped EAP based methods; if they are desired, they can be added back in

!       I added CancellationToken because it is probably worth it. If you don't want it you can remove it.
+       public Task<PingReply> SendPingAsync(IPAddress address, TimeSpan timeout, byte[]? buffer = null, PingOptions? options = null, CancellationToken cancellationToken = default);
+       public Task<PingReply> SendPingAsync(string hostNameOrAddress, TimeSpan timeout, byte[]? buffer = null, PingOptions? options = null, CancellationToken cancellationToken = default);
    }
}

namespace System.Net.Sockets {
    public class NetworkStream : Stream {
+       public void Close(TimeSpan timeout);
    }

    public class Socket {
+       public bool Poll(TimeSpan timeout, SelectMode mode);
+       public static void Select(IList checkRead, IList checkWrite, IList checkError, TimeSpan timeout);
    }
}

namespace System.ServiceProcess {
    public class ServiceBase {
+       public void RequestAdditionalTime(TimeSpan time);
    }
}

namespace System.Threading.Tasks {
    public class Task {
+       public bool Wait(TimeSpan timeout, CancellationToken cancellationToken); 
    } 
} 

namespace System.Timers {
    public class Timer {
+       public Timer(TimeSpan interval);
    }
}

APIs that I don't know how to make better:
Most of these are adding properties that would simply delegate to the existing millisecond-based properties. In the case of get-only properties, I went for adding extension methods. I am not sure how these could be made better; maybe it's not worth adding them at all.

namespace System.IO {
    public abstract class Stream {
+       public TimeSpan ReadTimeoutTimeSpan { get; set; }
+       public TimeSpan WriteTimeoutTimeSpan { get; set; }
    }
}

namespace System.IO.Ports {
    public class SerialPort {
+       public TimeSpan ReadTimeoutTimeSpan { get; set; }
+       public TimeSpan WriteTimeoutTimeSpan { get; set; }
    }
}

namespace System.Media {
    public class SoundPlayer {
+       public TimeSpan LoadTimeoutTimeSpan { get; set; } 
    }
}

namespace System.Net.NetworkInformation {
+   public static class NetworkInformationTimeSpanExtensions {
+       public static TimeSpan GetPacketReassemblyTimeout(this IPGlobalStatistics statistics);
+       public static TimeSpan GetMaximumTransmissionTimeout(this TcpStatistics statistics);
+       public static TimeSpan GetMinimumTransmissionTimeout(this TcpStatistics statistics);
+   }
}

namespace System.Net.Sockets {
    public class Socket {
+       public TimeSpan ReceiveTimeoutTimeSpan { get; set; }
+       public TimeSpan ReceiveTimeoutTimeSpan { get; set; }
    }

    public class TcpClient {
+       public TimeSpan ReceiveTimeoutTimeSpan { get; set; }
+       public TimeSpan SendTimeoutTimeSpan { get; set; }
    }
}

namespace System.Timers {
    public class Timer {
+       public TimeSpan IntervalTimeSpan { get; set; }
    }
}

APIs which use seconds:
These suffer from the same problem as the previous group, but also only deal with seconds, meaning that most TimeSpan values would need to be rounded out.

namespace System.Data {
!   This would need to be a DIM.
    public interface IDbCommand {
+       TimeSpan CommandTimeoutTimeSpan { get; set; }
    }

+   public static class DataTimeSpanExtensions {
+       public static TimeSpan GetConnectionTimeout(this IDbConnection connection);
+   }

}

namespace System.Data.Sql {
    public sealed class SqlNotificationRequest {
+       public TimeSpan TimeoutTimeSpan { get; set; }
    }
}

namespace System.Data.SqlClient {
    public sealed class SqlBulkCopy {
+       public TimeSpan BulkCopyTimeoutTimeSpan { get; set; }
    }

    public sealed class SqlConnectionStringBuilder {
+       public TimeSpan ConnectTimeoutTimeSpan { get; set; }
+       public TimeSpan LoadBalanceTimeoutTimeSpan { get; set; }
    }

    public sealed class SqlDependency {
+       public SqlDependency(SqlCommand command, string options, TimeSpan timeout);
    }
}

namespace System.Net.Sockets {
    public class LingerOption {
+       public LingerOption(bool enable, TimeSpan time);
+       public TimeSpan LingerTimeSpan { get; set; }
    }

    public class Socket {
+       public void Close(TimeSpan timeout);
    }
}

I omitted *WebRequest/ServicePoint/SmtpClient since we aren't innovating in that space. Additionally, I omitted the SynchronizationContext API because the docs mention a CLS-compliant alternative API which does take a TimeSpan.

[joperezr]

@reflectronic thanks for providing the list, that is very handy.

@terrajobst regarding your comment:

There is a lot of trickery due to .NET Framework compat and how we type forward contracts.

Is there anything impeading us adding these overloads just in netcoreapp? In case that's fine, then I guess this issue might be ready to take to API Review.

[terrajobst]

LOL. No idea what I meant. It was a different time back then. Maybe I thought how we can OOB these methods? Who knows. Let’s just look at the APIs in one of our next reviews. Seems generally like a fine feature.

[joperezr]

sounds good, will mark it as ready for review then.

[reflectronic]

Er... there wasn't actually an API proposal before. However, I have now updated my original comment with an API proposal.

[drieseng]

@jogibear9988 There are already APIs that expect a TimeSpan representing -1 milliseconds to indicate infinity.
One such example is System.Threading.Thread.Sleep(TimeSpan timeout).

As @huoyaoyuan mentioned, this value is currently exposed through System.Threading.Timer.InfiniteTimeSpan.

[danmoseley]

Assigning to @deeprobin

[danmoseley]

Hmm, @deeprobin I can't assign to you unless you're either an org member (which you aren't) or you've commented on the issue. If you comment here, I can assign it.

[deeprobin]

@danmoseley
Oh thanks for the info Dan. Here is my comment.

I did apply to the Foundation a while back, but I don't know at what intervals they look at their applications 😄.

[deeprobin]

@danmoseley
I think we could mark the issue as api-suggestion and let the 2nd group review already.

Currently only the 1st group is approved (See #14336 (comment)). Then I can take care of the other groups in a follow-up PR.

[danmoseley]

I did apply to the Foundation a while back, but I don't know at what intervals they look at their applications

That's different -- I'm talking about a Github security group that merely gives you the right to be assigned without a comment. If this keeps coming up, we can add you, but most contributors aren't in it.

I think we could mark the issue as api-suggestion and let the 2nd group review already.

I suspect that would become confusing. I suggest to open a new api suggestion issue for that 2nd group and copy over the relevant info. Then your PR can close this one.

[bartonjs]

I suggest to open a new api suggestion issue for that 2nd group

Yes, please 😄.

[deeprobin]

I suspect that would become confusing. I suggest to open a new api suggestion issue for that 2nd group and copy over the relevant info. Then your PR can close this one.

@danmoseley

I have created an issue for the 2nd group. Should I create one for the 3rd as well?

[danmoseley]

@deeprobin I suggest putting everything remaining into that one issue.