Memory/Thread Leak

[christianh104]

Describe the bug
When a connection is closed, a thread remains open (NIO-ELT-X-#0, where X increases with each connection). If a connection is opened and closed over and over, the thread count and memory usage just keep increasing, until eventually a DNS error occurs on the 249th connection attempt.

Reproducer Sample
main.swift (must be run outside of sandbox to read private key):

import CryptoKit
import Foundation

let username = "christian"
let host = "REDACTED"
let port = 22
let privateKey = "/Users/christian/.ssh/id_rsa"
var counter = 1

while true {
	print("attempt #\(counter)")
	counter += 1
	
	let privateKeyFileContents = try! String(contentsOf: URL(fileURLWithPath: privateKey), encoding: .utf8)
	let authenticationMethod:SSHAuthenticationMethod
	
	if let privateKey = try? Insecure.RSA.PrivateKey(sshRsa: privateKeyFileContents) {
		authenticationMethod = .rsa(username: username, privateKey: privateKey)
		print("Loaded RSA key")
	} else if let privateKey = try? Curve25519.Signing.PrivateKey(sshEd25519: privateKeyFileContents) {
		authenticationMethod = .ed25519(username: username, privateKey: privateKey)
		print("Loaded Ed25519 key")
	} else if let privateKey = try? P256.Signing.PrivateKey(pemRepresentation: privateKeyFileContents) {
		authenticationMethod = .p256(username: username, privateKey: privateKey)
		print("Loaded P256 key")
	} else if let privateKey = try? P384.Signing.PrivateKey(pemRepresentation: privateKeyFileContents) {
		authenticationMethod = .p384(username: username, privateKey: privateKey)
		print("Loaded P384 key")
	} else if let privateKey = try? P521.Signing.PrivateKey(pemRepresentation: privateKeyFileContents) {
		authenticationMethod = .p521(username: username, privateKey: privateKey)
		print("Loaded P521 key")
	} else {
		fatalError("Failed to read private key: Invalid key type")
	}
	
	let ssh = try! await SSHClient.connect(
		host: host,
		port: port,
		authenticationMethod: authenticationMethod,
		hostKeyValidator: .acceptAnything(),
		reconnect: .never
	)
	let sftp = try await ssh.openSFTP()
	// try? await sftp.close()
	try? await ssh.close()
	
	sleep(1) // Give the runtime a chance to clean up?
}

Client:

• OS: macOS Sequoia 15.0.1
• Client: Citadel
• Version of Citadel: 0.8.0

Server:

• OS: Debian
• Server: opensshd

Additional context
I've never touched Swift before this week, so I may be doing something wrong, but the repro seems pretty straightforward.
I wondered if the SFTP channel wasn't cleaning up, so I tried adding this to SFTPClient.swift:
public func close () async throws { try await self.channel.close() }
and then uncommenting try? await sftp.close() in main.swift. No luck.

Side note: If there is a better way to determine the private key type than brute force, I am interested to hear it.

[Joannis]

Hey, thanks for the issue. Let me answer them in order:

1. I'll run the reproducer and get back. I'm half-assuming the bug is that RSA isn't cleaned up - but not certain yet.
2. There is a way for me to give you a "neutral" OpenSSH key, that you can parse the key out of. That sounds very helpful for many scenarios - so consider it on the agenda
3. SFTP does close, but it's currently an open issue SFTP close function not found #70 that I'm resolving with Allow closing the SFTP channel, and provide a with-style method as well #75, to get a warning gone. Please do try that PR out

[qihuijia]

After I use #75 , I got an error:
'async' call cannot occur in a defer body. for

try? client.close()

[christianh104]

@qihuijia Just looking at it on my phone, it does appear that SFTPClient.close() is an async function, and SSHClient.withSFTP() calls it without await. It's also in a defer block, which doesn't support async function calls in Swift. I've got a month of Swift experience, never actually used defer myself, and have not tried the PR yet, so I must be missing something, but I feel like that shouldn't compile. All that said, it's completely unrelated to this issue, you should definitely open a new issue for that.

@Joannis Sorry I did not see your response sooner, I will definitely give the PR a go asap. I also forgot to mention that Xcode's memory leak detection was no help here. I will give this a go with password authentication to rule out RSA as the issue.

Also a heads up, I may have some PRs coming your way soon as I have noticed a few issues in the library, and a few features that would be nice to have. I don't have much experience with Git and writing PRs so I may need a bit of help when the time comes.

[Joannis]

@qihuijia apologies for that! I had a couple PRs that I think individually worked, but I should've tested once more with all changes together.

[Joannis]

@christianh104 and @qihuijia the close() call was fixed this morning (my time). The defer statements can't be async, but some close() calls exposed by NIO can be synchronous so don't run into this limitation. The issue is that I exposed it as asynchronous API to return potential errors.

@christianh104 I waited a bit for feedback, but started getting more and more questions about it. The error is definitely not wanted, so I fixed that in those two PRs which are now merged and usable as of 0.9.1

[Joannis]

I'm closing this issue since it should be all good now. Please do re-open if something is missing.
If you need help making PRs, join the Discord server or reach out to other mediums. Github is fine too, but make it a draft PR if it's unfinished. Definitely assign me for reviewer regardless, as I have a tool that tracks open PRs I'm involved with as a reviewer

[qihuijia]

@qihuijia apologies for that! I had a couple PRs that I think individually worked, but I should've tested once more with all changes together.

Thanks very much. The fix is very fast. It works for me. Have a nice day