Skip to content

Conversation

@nikolamilekic
Copy link
Contributor

AsTimeout is called from the SshCommand constructor with Timeout.InfiniteTimeSpan. In this scenario the range check should never fail, but unfortunately it does in certain scenarios, due to a runtime or compiler bug (as soon as optimizations are turned off the issue miraculously disappears).

Closes #1700

Copilot AI review requested due to automatic review settings October 7, 2025 11:51
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a rounding issue in the AsTimeout extension method that was causing incorrect timeout validation failures. The issue occurred when checking Timeout.InfiniteTimeSpan values due to floating-point precision problems that were compiler/optimization dependent.

  • Changed timeout calculation from double to long to avoid floating-point rounding issues
  • Updated the range check to use integer comparison instead of floating-point comparison
  • Enhanced the exception message to include the actual timeout value for better debugging

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@nikolamilekic
Copy link
Contributor Author

It can be argued that the range check is completely unnecessary when called from the SshCommand ctor (as it is always called with Timeout.InifiteTimeSpan), and that the _commandTimeout field should be set in the constructor directly (instead of going through the CommandTimeout property, which triggers the range check). I didn't want to propose this change right away, as I'm unsure if your coding guidelines allow it.

AsTimeout is called from the SshCommand constructor with
Timeout.InfiniteTimeSpan. In this scenario the range check should never
fail, but unfortunately it does in certain scenarios, due to a runtime
or compiler bug (as soon as optimizations are turned off the issue
miraculously disappears).

Closes sshnet#1700
@Rob-Hague
Copy link
Collaborator

I had half-fixed the tests that were asserting too strictly against the original commit, so I finished that and reverted to the original. Making the comparison on long rather than double feels safest for this problem

Thanks!

@Rob-Hague Rob-Hague merged commit c5c6f28 into sshnet:develop Oct 8, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ArgumentOutOfRangeException when verifying timeout range in SshCommand ctor

2 participants