theophilusx / ssh2-sftp-client

a client for SSH2 SFTP
Apache License 2.0
797 stars 195 forks source link

Opened too much file descriptors after "Write stream error" in _put() function. #543

Open Happy83 opened 2 days ago

Happy83 commented 2 days ago

Hi guys, I have a problem with bad permissions on target directory for function _put(). I receive message "_put: Write stream error: Permission denied" and the readStream for a local file isn't destroyed. And then I see a many opened file descriptors and I get “Too many open files” error message in linux.

I think, the right solution is to destroy readStream in wtr.once('error', ... block.

What do you thinks?

theophilusx commented 2 days ago

If you are talking about a read stream passed in by a client script, I don't think it should be destroyed by this module. Rather, it is the responsibility of the client script to manage this resource. It is quite possible there are scenarios where the client library may want to do something other than close the stream when an error occurs. Note also that the put method does not destroy the stream after a successful upload either as again, management of that resource is the responsibility of the client code. The ssh2-sftp-client module merely uses that resource to get the data for the put.

If you are talking about the read stream crated inside the put method i.e. when it is called with a file path source string, that read stream is destroyed when the method exits.

Happy83 @.***> writes:

Hi guys, I have a problem with bad permissions on target directory for function _put(). I receive message "_put: Write stream error: Permission denied" and the readStream for a local file isn't destroyed. And then I see a many opened file descriptors and I get “Too many open files” error message in linux.

I think, the right solution is to destroy readStream in wtr.once('error', ... block.

What do you thinks?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.

Happy83 commented 1 day ago

Yeah, I'm talking about the read stream created inside the put method. When the write stream is rejected with error 'Write stream error', the read stream isn't destroyed.

I simulated this behavior with a simply loop every 5 seconds. I wanted to put the file from local computer to remote storage with no write permission to destination folder. Everytime I received message '_put: Write stream error: Permission denied'. That's ok. But...

When I call 'll /proc/[pid]/fd' I see a lot of rows with the source file. Every 5 seconds increased by new opening for reading.

So, I tried to add this code after your wtr.once('error', (err) => { on row 715 (index.js):

if (rdr) {
    rdr.destroy()
}

And voila, it's worked. No one new file descriptor stay opened.

theophilusx commented 1 day ago

OK, I will habe a look at it when i get time. The solution will be slightly more complicated as the library should not close the stream if it is a stream passed in as opposed to one created within the method.