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

Thread pool exhaustion due to MessageListener running on ThreadPool #824

Closed
IgorMilavec opened this issue May 14, 2021 · 3 comments · Fixed by #902
Closed

Thread pool exhaustion due to MessageListener running on ThreadPool #824

IgorMilavec opened this issue May 14, 2021 · 3 comments · Fixed by #902

Comments

@IgorMilavec
Copy link
Collaborator

Session.Connect() (and Session.ConnectAsync() in #819) start MessageListener through ThreadAbstraction that runs the connection message pump on a ThreadPool thread. This causes thread pool exhaustion when the number of connected sessions reaches the maximum number of thread pool threads. Message loop should not be run through thread pool, as this consumes a (limited) shared resource. Also, running a thread for each session impacts system performance and limits scalability of the application.
Preferably we should avoid consuming an entire thread and make the message loop async.

@vivek4434
Copy link

@IgorMilavec, Could you please help me in understanding how you were able to conclude that ThreadPool was exhausted?

@IgorMilavec
Copy link
Collaborator Author

IgorMilavec commented Jun 28, 2021

I detected thread pool exhaustion the old fashioned way: my service stopped performing, but no errors were detected/reported. Upon investigation I determined that ThreadPool.GetAvailableThreads() reported lower and lower numbers, eventually consuming all of them. Did I mention my service runs in an environment that limits the maximum number of thread pool workers ;)? Traces had shown correlation between establishing new SFTP session and getting lower available threads in the thread pool.

I saw that in Session.Connect() the Session.MessageListener() was started using ThreadAbstraction.ExecuteThread(), which in turn called ThreadPool.QueueUserWorkItem(). Because Session.MessageListener() continues running/blocking for the duration of the session, the thread would be consumed from the thread pool for the duration of the session. Just for proof I changed Session.Connect() to start Session.MessageListener() using ThreadAbstraction.ExecuteThreadLongRunning(), which runs it on a new thread. This eliminated the thread pool exhaustion in my service. Please note that now you need one OS thread per SFTP/SSH connection, which still limits the scalability of the solution. So the above is only a workaround.
One more thing that might be related to your problem in #840: when thread pool detects that all its threads are in use, it only adds one new thread per second to the pool. This limits the number of new SFTP/SSH connections to one per second.

For scenarios where you have a reasonable amount of short lived connections, the current solutions performs better. For scenarios where you have a lot of long lived connections, my workaround allows the system to run at all, but at the cost of system performance. By making the Session.MessageListener() async we would get the solution that would scale for both scenarios.

@drieseng
Copy link
Member

I see that you now use ThreadAbstraction.ExecuteThreadLongRunning(...) in Session.ConnectAsync(CancellationToken cancellationToken). Please submit a PR to also fix this for Session.Connect().

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