This tests ReadFrom's determining concurrency from the estimated input
size. go test -cover -integration, before:
coverage: 77.3% of statements
After:
coverage: 79.2% of statements
Previously, these bits were ignored by Chmod, which sent the numerical
value of the mode argument as-is to the server. As a result, callers had
to supply POSIX values for setuid, setgid and sticky to Chmod.
The new version supports both the POSIX values and the Go values.
Also added a note to the docs to clarify that the umask is not
subtracted from the mode, and why that is. The only portable way to get
the umask is to set it, then reset it, but that's racy. On Linux, we
could parse /proc/self/status, but that doesn't work portably and will
fail where /proc is not available (some Docker containers, notably).
Many Unix assuptions have been made when testing.
Previously running tests on windows threw a panic
part way through the tests. After these changes
many more tests pass when the fix was isolated
to the test itself, and none panic.
Some tests are skipped because they do make sense
on windows (chmod, chown), while others are skipped
just to how the test was implemented.
Lastly, some of the external executables were hard coded.
Change these to look paths in TestMain first. This is done
to better support windows, where openssh may be installed and
sftp and sftp-server may both be in the PATH.
Fixes#234
Similar to #181 (which was about read methods), the client deadlocks on
server drop on write methods (Write/ReadFrom). This is another case of
broadcastErr() and dispatchRequest() deadlocking.
The fix is to bump up the channel used to communicate about inflight
packets to maxConcurrentRequests+1 (+1 is new). It seems that it can
have the full set of packets going and hit the error (triggering
broadcastErr) yet it still tries to call dispatchRequest again. More
details in that ticket.
I'm also bumping up the chan buffer in the read methods to keep them
consistent. I can't reproduce the problem, but it looks like it should
have it. So it might just be a much harder race to trigger.
This also includes 2 tests which reprocuded the issue for ReadFrom and
Write.
to support actual rename() operations rather than the link() call
effectively mandated by the sftp v3/draft-2 requirement that targets
not be overwritten.
match contained lots of cut-n-pasted code from filepath/match.go in the
core libraries. Particularly Match() and Join() were changed in such a
way that made them functionally equivalent to the versions in
path/match.go. This removes the duplicate code and just calls the path
versions.
The client was deadlocking if you killed the server when a file transfer
was occuring. The problem occured when you had many requests sent and
were waiting on responses when the server dies. The errors for all those
pending requests get sent along the response channel which is also used
for the errors reported by the new requests being made. The new request
channel send is in the same loop as the channel reading, so when it
blocked due to all other errors filling the channel it deadlocked the
program.
There were 2 possible fixes, changing the new request error channel
usage to be in a separate goroutine to keep it from blocking the loop or
to increase the size of the buffer to handle all the errors. This
implements the latter.
Included is a test that reproduces the problem most of the time. Due to
the required timing of the issue, it was impossible to reproduce 100% of
the time.
benchmarks were broken due to double closing of a channel in some tests.
I just commented out the problematic closes which probably means it's
leaking in the tests but at least they run now. Should probably try to
figure out what happened here at some point.
github issue #145
A deadlock will occur in cases when using client.Write or client.ReadFrom,
it has 2 network issues in a row, and it is starting a new loop to send
more concurrent packets.
Cassondra Foesch
Gleb Teterin
greatroar
Cassondra Foesch
codesoap
greatroar
Nicola Murino
Tommie Gannert
Daniel Theophanes
Urjit Singh Bhatia
John Eikenberry
John Eikenberry
Iain Wade
Catalin Tanasescu