Skip to content

Infinite timeout on async UploadFile with OpenAsync/CopyToAsync #1497

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
Dunge opened this issue Sep 19, 2024 · 3 comments · Fixed by #1510
Closed

Infinite timeout on async UploadFile with OpenAsync/CopyToAsync #1497

Dunge opened this issue Sep 19, 2024 · 3 comments · Fixed by #1510

Comments

@Dunge
Copy link

Dunge commented Sep 19, 2024

I'm using the code here #819 (comment) to have an async upload file and it works.

Unfortunately, if I'm uploading a file and close the connection on the remote end, the method will never finish. I would be expecting some exception to be thrown. Instead the thread remains stuck indefinitely, and it's not a good thing for server-side process doing this operation in batches.

I found this decade old question and tried to set the OperationTimeout property to 30sec, but it doesn't seems to have any effect.

I'm aware I can have a CancellationToken, but this would be a way for me to trigger a cancellation from my end, not one that happens when the connection get closed remotely or if a timeout happen.

@Rob-Hague
Copy link
Collaborator

I think the problem is that the non-async methods in SftpSession are checking on a few wait handles which can throw errors:

WaitOnHandle(wait, OperationTimeout);

public void WaitOnHandle(WaitHandle waitHandle, int millisecondsTimeout)
{
var waitHandles = new[]
{
_errorOccuredWaitHandle,
_sessionDisconnectedWaitHandle,
_channelClosedWaitHandle,
waitHandle
};

But the async methods are not. They should at least (do something like) subscribe to the same events where those wait handles are set

@Dunge
Copy link
Author

Dunge commented Sep 21, 2024

Thank you very much for this first analysis, you seems to be on to something.

At least it confirms that it's not just me using the lib incorrectly or missing a configuration parameter somewhere.

I assume it's not as simple as adding that same call in the async methods since it's using streams and probably can't subscribe to events the same way. I would love to continue on your investigation work and submit a PR, but I unfortunately do not trust my knowledge enough to fix it properly.

@Rob-Hague
Copy link
Collaborator

No problem, yeah I don't think it's quite that simple but I might have a solution which is not too dissimilar

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 a pull request may close this issue.

2 participants