Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

File handles still open during a download on_close event #21

Open
acoulton opened this Issue · 1 comment

2 participants

@acoulton

Net::SFTP::Operations::Download fires the on_close event to indicate that a file has finished transfer before it closes the file handles - see L329

if response.eof?
    update_progress(:close, entry)
    entry.sink.close
    request = sftp.close(entry.handle, &method(:on_close))

On Windows at least, this makes it difficult to use the on_close event to set file modification time, as the operating system gives an access denied error if the local file is open. There may be other similar problems for other kinds of local file access during the download.

It would be great if the sequence of this code could change so that handles are closed before the event. However, I realise that might cause problems for existing third-party event handlers that expect the file to be open.

An alternative might be to allow the event handler to close the file handle if required - currently if you call entry.sink.close in the event handler there is then an IO error once control returns to Net:SFTP and it tries to close it again.

I'd be happy to submit a patch and tests if you indicate which approach you'd prefer.

@delano
Collaborator

Nice catch. I agree that logically it makes more sense to close the file before the on_close event. I'd suggest to make the change and add a test for that specific case (updating modification time on_close). As long as the existing tests pass I'll push it out. If not we'll take another look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.