Create a master Context in the per-session ReqeustServer.Serve method
that is then used as the parent for all Context's created in that
server. Its cancelFunc is then always called when Serve exits. This is
in line which out the http.serve code works.
Each Request gets a child WithCancel context created in the
requestFromPacket call. This Context is still closed on request.close().
Recent refactoring has eliminated the need for Fstat and Fsetstat (stat
operations on file handle instead of path)to be special cased. They will
now just work with the normal case for any packet with a file handle.
Fixes#217, where the path cleaning code was mangling the volume name on
windows; ie. "c:/some/path" was turned into "/c:/some/path". Using the
filepath.IsAbs() (vs path.IsAbs()) uses OS specific tests and fixes
this.
This goes against strict POSIX paths, but this used to work and fixing
it breaks nothing on other platforms. And we have still standardized on
POSIX style delimiters.
When the SSH_FXP_OPEN packet has the SSH_FXF_CREAT pflag set the file
should get created even if no data is sent. We do this by having the
Open method call through to the FilePut Handler with empty packet data.
Fixes#214
In #192 we normalized all Request use to have pointer receivers. This
simplified reasoning and allowed for a simpler Request struct. This
change completes that work by eliminating the need for the
state/stackLock pointers and also removes the need for a new Request
each packet, reducing the pressure on the GC.
The main bit is that the RequestServer.openRequests[] map now stores Request
pointers and it only creates new Requests (replacing the mapped entry) when the
Method changes. It never updates/modifies an existing Request and the packet to
Method mapping is now all in place place (requestMethod).
Fix#207
The primary issue of the resolved ticket was path cleaning code returned
the path of '/.' instead of '/'.
We also reviewed and clarified the use of filepath/path and how OS
specific paths are handled in the server code. All paths are converted
to POSIX paths as they come into the server. They are then POSIX paths
for all internal operations.
Request object had a packet channel where it stored packets for
read/write/readdir handling. It was required in an earlier version of
the code (that was refactored before merging) and never got cleaned up.
This removes that channel and just passes the packet through the methods
instead. This is much simpler and would have eliminated the chance of
issue #195. It also looks like it should allow for a few more
simplifications as well.
TLDR; have the Handler wrappers generate the response packets for the
errors instead of returning the errors.
Errors from the handers was returned up the stack to the request-server
handler call. In cases of errors the packet for the error was then
generated using the passed in packet (the incoming packet).
For file read/write/list operations the incoming packet data (includeing
ID) is put in a queue/channel so that it can be handled by the same file
handling code returned by the handler.
Due to gorouting scheduling, in some cases the packets in the channel
won't line up with the incoming packets. That is the worker that took an
incoming packet with one ID might respond with a packet with a different
ID. This doesn't matter as the reply order of packets doesn't matter.
BUT when this response packet that doesn't match IDs with the incoming
packet system returns an error, it uses the incoming packets ID to
generate the error packet. So the error response would use that ID and
the packet from the queue, once processed, would also use that ID
(because on success it uses the ID from the packet in the queue).
This patch fixes the issue by having the code that works with the
packets, either incoming or from the queue, generate the error packet.
So there is no chance of them getting out of sync.
Splitted cleanPath into cleanPacketPath and cleanPath for better handling of slashes in file paths
Added test for cleanPath func
Removed code duplication => filepath.ToSlash(filepath.Clean(...)) => cleanPath(...)
Fixed tests for runLs to match year or time
Renamed constants to fit hound rules
I didn't like how the initial fix for #184 required the handler author
to track the token/offset for the filelist. I came up with a way to do
it that mimic'd the FileReader handler interface using a method
something like ReaderAt except for file lists.
changed MaxFileinfoList to a var for testing and tweaking
Renaming FileInfoer interface to FileLister.
FileInfoer -> FileLister
Fileinfo -> Filelist
There is a data race with the waitgroup (wg) object used to synchronize
the workers with the server exit. The workers called wg.Add()
asynchronously and it was possible for the Wait() to get hit before any
of the Add() calls were made in certain conditions. I only ever saw this
sporatically in the travis tests.
This fixes it by making the wg.Add() calls synchronous.
Handle fsetstat in same was a fstat, by pulling path from stored file
info and processing in that form. Makes handlers simpler while
preserving functionality.
Support fstat by converting to a standard Stat call and processing that.
I didn't want to add another special case to the request backend, I
wanted to keep it only having to support Stat.
Server struct contains Mutex which might be copied in inconsistent
state. Avoid this by declaring methods on pointer receiver.
This calms down go race detector.
Signed-off-by: Pavel Borzenkov <pavel.borzenkov@gmail.com>
Encapsulate stateful data into sub-structures with pointer references
from the Request structure. This allows me to pass by value in most
cases to keep non-stateful (write once) data data race free and tightly
controlling access to stateful data to ease locking.
Was using a filename as a placeholder for something that would allow for
stateless servers but decided the amount of work wasn't worth it at this
point. So I went back to the auto-inc number as it doesn't have the size
issues (max handle length is 256).
Allan Feid
John Eikenberry
Mayank Agarwal
sandreas
Pavel Borzenkov