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.
netPipe provides a pair of io.ReadWriteClosers connected to each other.
The functions is identical to os.Pipe with the exception that netPipe
provides the Read/Close guarentees that os.File derrived pipes do not.
- no need to initalise mutexes, their zero value is valid
- make NewServer simpler, 90% of use cases already had an
io.ReadWriteCloser, in the single case that it it is easy to provide a
simple wrapper.
Fixes#69
io.Pipe is unbuffered and provides a poor replacement for a net.Conn, use an os.Pipe instead.
Also skip some tests which don't work under the Go server as it runs in process.
greatroar
Nicola Murino
Cassondra Foesch
Tommie Gannert
Daniel Theophanes
Urjit Singh Bhatia
John Eikenberry
John Eikenberry
Iain Wade
Catalin Tanasescu
Dave Cheney
Mark Sheahan
Matt Layher
Matthew Sykes
DrJosh9000