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

scp starts to throw "too many files" with large recursive transfer #1209

Closed
cbookg opened this issue Jul 6, 2018 · 5 comments
Closed

scp starts to throw "too many files" with large recursive transfer #1209

cbookg opened this issue Jul 6, 2018 · 5 comments

Comments

@cbookg
Copy link

cbookg commented Jul 6, 2018

Please answer the following
"OpenSSH for Windows" version

  • v7.6.1.0p1-Beta
  • also reproduced after building scp from head.

Server OperatingSystem
Linux - Ubuntu

Client OperatingSystem
Windows 10 Enterprise

What is failing
Steps:

  • Run "scp -r * targethost:/target/path" in a directory structure containing 8500 files.

Expected output

  • All files transfered properly.

Actual output

  • The last 50 files fail to transfer. The output looks something like this:
    ...
    filethatworks1 100% 6192 6.1KB/s 00:00
    filethatworks2 100% 894 0.9KB/s 00:00
    filethatfails1: eother
    filethatfails2: eother
    ...

I reproduced this by building from head and setting breakpoints to figure out more details about the failure.
The errors are coming from the win32 specific fileio_fstat() function. The call to _open_osfhandle returns 24, which appears to mean "too many open files".

It appears as though scp is leaking file handles or something. Is this a known issue?

@bagajjal
Copy link
Collaborator

bagajjal commented Jul 6, 2018

As per MSDN, _open_osfhandle returns -1 in case of failure. You mentioned it returned 24 which means it's successful and failing some where else?

@cbookg
Copy link
Author

cbookg commented Jul 6, 2018

Sorry, it does return -1. The value of errno when it does is 24.

@cbookg
Copy link
Author

cbookg commented Jul 11, 2018

The problem seems to be that this function is using _open_osfhandle which creates a new "runtime file descriptor", stored in fd but never closed (so it is leaked). We can't close this directly as the windows handle is still used outside of this function. ie from the docs "The _open_osfhandle call transfers ownership of the Win32 file handle to the file descriptor. To close a file opened with _open_osfhandle, call _close"

Instead, it looks like we should duplicate the windows handle, and then ensure we close the fd created from _open_osfhandle after the _fstat64:

fileio_fstat(struct w32_io* pio, struct _stat64 *buf)
{
int fd = _open_osfhandle((intptr_t)pio->handle, 0);
debug4("fstat - pio:%p", pio);
if (fd == -1) {
errno = EOTHER;
return -1;
}

return _fstat64(fd, buf);

}

@NoMoreFood
Copy link

@bagajjal His change looks good to me.

@manojampalam
Copy link
Contributor

manojampalam commented Jul 12, 2018

@cbookg thanks for following up. I'll pull in your fix.

manojampalam pushed a commit to PowerShell/openssh-portable that referenced this issue Jul 13, 2018
Fix descriptor leaks in win32 fstat implementation
PowerShell/Win32-OpenSSH#1209

According to the docs for _open_osfhandle, _close will close the underlying handle:
"To close a file opened with , call _close. The underlying handle is also closed by a call to _close, so it is not necessary to call the Win32 function CloseHandle on the original handle"
manojampalam pushed a commit to manojampalam/openssh-portable that referenced this issue Oct 2, 2018
…dress VS warnings.

Includes:
- Fix descriptor leaks in win32 fstat implementation: PowerShell/Win32-OpenSSH#1209
- Modified user principal name lookup to default to the implicit form (SamAccountName@DnsDomainName) if no explicit user principal name attribute is found on the account: PowerShell/Win32-OpenSSH#1213
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

5 participants