Skip to content
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

Crashing host process on socket timeout #1690

Open
kenkendk opened this issue Dec 9, 2024 · 4 comments
Open

Crashing host process on socket timeout #1690

kenkendk opened this issue Dec 9, 2024 · 4 comments

Comments

@kenkendk
Copy link

kenkendk commented Dec 9, 2024

FTP Server OS: Linux

FTP Server Type: Unknown

Client Computer OS: Linux

FluentFTP Version: 50.1.0

Framework: .NET8

During a timeout it looks like the shutdown process causes an unhandled exception.

I think the cause is this line:
https://github.com/robinrodricks/FluentFTP/blob/master/FluentFTP/Streams/FtpSocketStream.cs#L560

In here it will attempt to close the connection due to a timeout, but if the socket is already faulty, the close call will fail as well.
Since this happens in the token callback, the uncaught exception has nowhere to go.

Logs :

2024-11-28T10:01:04.428515378Z Unhandled exception. System.TimeoutException: Timed out trying to read data from the socket stream!
2024-11-28T10:01:04.428549934Z    at FluentFTP.FtpSocketStream.ReadAsync(Byte[] buffer, Int32 offset, Int32 count, CancellationToken token)
2024-11-28T10:01:04.428554434Z    at FluentFTP.FtpSocketStream.ReadLineAsync(Encoding encoding, CancellationToken token)
2024-11-28T10:01:04.428565138Z    at FluentFTP.Client.BaseClient.BaseFtpClient.FluentFTP.IInternalFtpClient.GetReplyInternal(CancellationToken token, String command, Boolean exhaustNoop, Int32 timeOut, Boolean useSema)
2024-11-28T10:01:04.428569601Z    at FluentFTP.Client.BaseClient.BaseFtpClient.FluentFTP.IInternalFtpClient.GetReplyInternal(CancellationToken token, String command)
2024-11-28T10:01:04.428572767Z    at FluentFTP.Client.BaseClient.BaseFtpClient.FluentFTP.IInternalFtpClient.CloseDataStreamInternal(FtpDataStream stream, CancellationToken token)
2024-11-28T10:01:04.428576045Z    at FluentFTP.Client.BaseClient.BaseFtpClient.FluentFTP.IInternalFtpClient.CloseDataStreamInternal(FtpDataStream stream, CancellationToken token)
2024-11-28T10:01:04.428579008Z    at FluentFTP.FtpDataStream.CloseAsync(CancellationToken token)
2024-11-28T10:01:04.428581767Z    at FluentFTP.FtpSocketStream.<>c__DisplayClass74_0.<<ReadAsync>b__0>d.MoveNext()
2024-11-28T10:01:04.428585082Z --- End of stack trace from previous location ---
2024-11-28T10:01:04.428587693Z    at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__128_1(Object state)
2024-11-28T10:01:04.428590767Z    at System.Threading.QueueUserWorkItemCallback.Execute()
2024-11-28T10:01:04.428593527Z    at System.Threading.ThreadPoolWorkQueue.Dispatch()
2024-11-28T10:01:04.428596693Z    at System.Threading.PortableThreadPool.WorkerThread.WorkerThreadStart()
@kenkendk
Copy link
Author

kenkendk commented Dec 9, 2024

I will see if the upgrade to 52.0.0 fares better. I can see that the close sequence was updated in the new version.
Note that the ReadAsync method that takes the Span<byte> input is still using the previous logic with the close happening in the CancellationToken.Register callback, where the byte[] method is updated.

I think there is still some logic related to handling the close, because a cancelled (outer) token will cause a different cancellation error than what is intended (will throw from within the close call, not the if statement later).
And in the case the socket is faulty, it will also throw a different exception, hiding the original error.

But at least the call chain is now preserved, and the exception should bubble to the caller.

@FanDjango
Copy link
Collaborator

Yes, please check this with V52.

As we should be ending up here:

https://github.com/FanDjango/FluentFTP/blob/0087db909a7e1ebac4b43e30cc8bbbb1f334ed1c/FluentFTP/Streams/FtpSocketStream.cs?plain=1#L1822C1-L1840C1

and I would be interested in how that "fares".

@kenkendk
Copy link
Author

It appears to still happen with 52.0.0, but I have not collected enough details yet:
duplicati/duplicati#5730 (comment)

@FanDjango
Copy link
Collaborator

FanDjango commented Dec 22, 2024

Ok, this one will be difficult. Stack trace might be helpful as a starter. I wonder about the exact order of events and methods called on this timeout. Oh, and we do now have some good logging in there also, so perhaps if you can get logs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants