Invoke-Restmethode won't timeout

[KrX3D]

Steps to reproduce

Hello,

i got this code here for demonstration:

Invoke-RestMethod -uri "https://XXXXXX.blob.core.windows.net/XXXXXXXX/XXXXXX/SOURCES/PE10.WIM?st=2020-02-20T13%3A08%3A43Z&se=2021-02-21T13%3A08%3A00Z&sp=racwdl&sv=2018-03-28&sr=b&sig=SIGNATURE" -OutFile "C:\Users\testUser\Downloads\DL\PRG\test.wim"

when i use that code i can get the download.
Firstly to maybe make clear what the parameter TimeoutSec should do. i thought it the connection stops i get an timeout after 10 seconds? or do i get an timeout after 10 seconds when i cant get an answer ?

To the problem: when i use that code above the download starts with Powershell 7. When i unplug my Lan cable when the download is already progressing, i can reattach the cable in maybe 10 seconds and the download resumes. when i leave it unpluged this happens:

in Powershell 5: i get an timeout after 5 minutes. and the script goes on (thats what i want)
in Powershell 7: nothing happens, waiting 5, 10 minutes, even hours (tested till 1 day) and its stuck. also the script is stuck and wont progress anymore

Expected behavior

• after the connection is lost during an download (or cable is unpluged) a timeout should happen after 5 minutes, like it happens in Powershell 5

# Actual behavior

- Powershell 7 is stuck during the download when cable is unpluged. Script hangs forever

Environment data

[iSazonov]

@KrX3D Please share PSVertionTable as issue template requested.
Also please test with latest build PowerShell 7.1 Preview1.

[KrX3D]

Ah sorry here it is for PS7 which does not work:

Name Value

PSVersion 7.0.0
PSEdition Core
GitCommitId 7.0.0
OS Microsoft Windows 10.0.14393
Platform Win32NT
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1
WSManStackVersion 3.0

And here for PS5 which does work:

Name Value

PSVersion 5.1.14393.3471
PSEdition Desktop
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0...}
BuildVersion 10.0.14393.3471
CLRVersion 4.0.30319.42000
WSManStackVersion 3.0
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1

[iSazonov]

Please check with 7.1 Preview1

[KrX3D]

i tried it with the preview version. Still the same problem:

Name Value

PSVersion 7.1.0-preview.1
PSEdition Core
GitCommitId 7.1.0-preview.1
OS Microsoft Windows 10.0.14393
Platform Win32NT
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1
WSManStackVersion 3.0

[KrX3D]

Hello,
anything new about this problem?

[iSazonov]

I think it is easy to implement.
The method

PowerShell/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/StreamHelper.cs

Line 278 in c22ccbe

internal static void WriteToStream(Stream input, Stream output, PSCmdlet cmdlet, CancellationToken cancellationToken)

accept a cancelation token so CancellationTokenSource could be initialized with a default timeout like 300 seconds.

[vexx32]

We should also ensure that -TimeoutSec is respected there as well, but a sensible default would be wise.

[iSazonov]

TimeoutSec is used in HttpClient as connection timeout.

[KrX3D]

ok, but still the Invoke-Rest Comand should timeout after 5 minutes in PS7 like it does in PS5 and not hang for eternity

[KrX3D]

i updated my first post and removed the timeout parameter.
but like i said, with no connection the invoke-restmethode function is stuck in powershell 7 (tested with 7.0.3) and in PS5 it just "timeouts" after 5 minutes and my script can go on

[KrX3D]

i tried it with PS 7.1 but still the same, the script "hangs" forever in the invoke-restmethode function and does nothing.

so if i download something and my router disconnects, PS stays at Invoke-Restmethode fir eternity

PSVersion 7.1.0
PSEdition Core
GitCommitId 7.1.0
OS Microsoft Windows 10.0.18363
Platform Win32NT
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1
WSManStackVersion 3.0

[CarloToso]

I think the problem starts here:

PowerShell/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs

Line 1255 in 4c586a7

response = client.SendAsync(req, HttpCompletionOption.ResponseHeadersRead, _cancelToken.Token).GetAwaiter().GetResult();

If i shut down the wifi while downloading a file (without -OutFile), the download hangs forever and I can't manually stop the process with Ctrl+C

[iSazonov]

@CarloToso I discussed this problem in the .Net Runtime repository. The problem is that the cancel token is used for this method, but not for the lower methods it uses (or rather it is not passed to the streams it uses). And the .Net team did not agree to change this.
Perhaps the only solution is to unload this request (client.SendAsync) into a separate Task that can be killed by timeout.

[kasini3000]

Perhaps the only solution is to unload this request (client.SendAsync) into a separate Task that can be killed by timeout.

Yes, powershell requires thread timeout.but:
#18027
PowerShell/ThreadJob#26

[stevenebutler]

If the _cancelToken has .CancelAfter(timeout) or is constructed with the timeout, and then passed to all methods that process the request stream then it would cancel either when ctrl-c is pressed or when timeout expires.

Currently the canceltoken is only used as far as sending the request and receiving the response headers and is then ignored in many methods that read the response data. Many of these methods don't accept a cancellation token, but it might be possible to make use of variants that do.

@iSazonov you stated the TimeoutSec is only used for the connection timeout. Should we add a second timeout for receiving data (e.g. ReceiveTimeout) with a reasonably high default (e.g. 5 minutes to make it similar to PS 5.1)? and pass that cancellation token to the stream handling routines?

[stevenebutler]

I have made changes to my local tree that works by passing the cancellation token through to all stream APIs.

I had to substitute use of synchronous APIs for asynchronous APIs and pass the cancellation token to those, and use GetAwaiter().GetResult() to make them synchronous again.

Because CTRL-C StopProcessing cancels the cancellationToken, this fixes issues where CTRL-C was being ignored inside the stream readers.

The response read timeout is handled by adding the code below following the acquisition of the HttpResponseObject around line 564 of WebRequestPSCmdlet.Common.cs . Because the token is scheduled to cancel after the request is sent, the TimeoutSec parameter is used as a send timeout then used again as a receive timeout (effectively resetting the timeout) before waiting for the read response to complete. It may be still be better to use independent timeout values for send/receive. For example, a big file may take a long time to download, but a user may only want a much shorter timeout on time to get the http response headers.

    if (TimeoutSec > 0)
    {
        // Set timeout for receiving the data from the HTTP response stream
        _cancelToken.CancelAfter(TimeSpan.FromSeconds(TimeoutSec));
    }

[iSazonov]

@iSazonov you stated the TimeoutSec is only used for the connection timeout. Should we add a second timeout for receiving data (e.g. ReceiveTimeout) with a reasonably high default (e.g. 5 minutes to make it similar to PS 5.1)? and pass that cancellation token to the stream handling routines?

I said about HttpClient implementation.
PowerShell docs say about the parameter:

Specifies how long the request can be pending before it times out

I take it that if during any operation any request hangs for this time (network loss), then the cmdlet should terminate.
The .Net does not seem to support this (at least in HttpClient).
So we could only implement Ctrl-C, the easiest way is to put everything into a new task with separate CTS.

[stevenebutler]

@iSazonov it should be possible to do this without using a separate task provided the cancellation token we already have is passed through to low levels. I believe my WIP PR already demonstrates this for the CTRL-C issue and will need some additional work to support timing out when no traffic is received for a period of time (TimeoutSec).

The reason HttpClient timeout is not in play anymore is because once data is being read from the stream directly, the HttpClient API is no longer being used. The streams APIs used to do this have cancellation support if the Async APIs are used, so it is possible to implement timeouts using the cancellation token while reading from the streams.

Because of this I don't think the CmdLet needs to, or even should use a separate task to support cancellation and timeouts.

If the task is cancelled it will return to the user, but unless the cancellation token also cancels the underlying operations, they will continue even though the cmdlet appears to be finished. If the cancellation token is passed down to those underlying operations instead, when the cancellation token is cancelled it will abort the underlying operations.

The trick with this issue is that rather than using a straight timeout, the cancellation token timeout needs to be reset whenever data is received on the connection. If the cancellation token source is available to the underlying layers that are reading from the socket, it is possible to reset the timeout every time data is read by calling cts.CancelAfter(timeoutVal).

The second problem with using a task in the cmdlet is it will not be able to write progress or output to the stream from that task directly since it will likely be running in a separate thread and PowerShell does not support using WriteXXX APIs outside the main thread if I recall correctly.

I haven't tried to implement this as I'd like to work on PR #19315 which is simpler and fixes the CTRL-C support first before implementing a fix for this issue in the interests of keeping the PR small and easier to review.

[iSazonov]

The problem is that when we pass a CTS into the .Net API it does not guarantee that this CTS will work as we expect, because there can be many layers within that API.
In particular I discussed this problem for the HttpClient in the .Net Runtime repository. But the same problem could be with any other API.
So it doesn't make sense to convert the code to async and distribute CTS. Eventually it will fix all the problems and will unpredictably depend on the implementation in .Net.
What we can do is explicitly solve the problem at hand. In particular, add support for Ctrl-C.

[stevenebutler]

The .NET APIs that take cancellation tokens do work with the cancellation token. The issue you're describing is that HttpClient and handler, take timeouts properties but don't cancel the request when they are no longer effectively controlling them because they have handed off the response stream to be read independently. The HttpClient is not providing cancellation token to the underlying stream to cancel them because it is not reading them. The streams APIs take the cancellation token in the supported API calls and don't save a reference to the token so if they are read without passing a token they cannot be cancelled. When reading the streams if no cancellation token is passed to them, they have no way of being cancelled.

If a separate task is used, how would the supervisor then monitor the stream for activity so it can reset the cancellation timeout whenever traffic is received? The requirement is for a timeout on no stream activity over a period of time (the TimeoutSec), not a straight-forward timeout from the start of the response.

.NET core onwards is heavily biased towards cancellable asynchronous APIs so we should take advantage of them where it makes sense.

Please have a look at PR#19330 which uses the cancellable APIs to make CTRL-C work. With some additional logic to reset the cancellation token timeout whenever data is read from the stream it should be possible to do similar for this issue.

[iSazonov]

The .NET APIs that take cancellation tokens do work with the cancellation token.

They work. But not always as you expected. See dotnet/runtime#74568
In other words, your PR does not solve the Ctrl-C problem completely.
And if so, it is better not to complicate the code and apply a more universal solution.

[ghost]

🎉This issue was addressed in #19558, which has now been successfully released as v7.4.0-preview.4.:tada:

Handy links:

• Release Notes