Add LstatFileLister, an optional interface for FileLister, if this
interface is implemented Lstat requests will call it otherwirse they
will be handled in the same way as Stat, as before
after processing a packet we keep in memory the allocated slices and we reuse
them for new packets.
Slices are allocated in:
- recvPacket
- when we receive a sshFxpReadPacket (downloads)
The allocated slices have a fixed size = maxMsgLength.
Allocated slices are referenced to the request order id and are marked for reuse
after a request is served in maybeSendPackets.
The allocator is added to the packetManager struct and it is cleaned at the end
of the Serve() function.
This allocation mode is optional and disabled by default
When fstat is called it now uses the handle to get the opened request,
pulls the filepath from that and creates a new request for the fstat
call. This eliminates the need to change the Method on the request
object and furthers the goal of eliminating post-creation mutation of
the request object to open up the possibility to eliminate or at least
reduce the use of the global request lock.
Just like with files, the call to the hander to create the Lister is
done when the OPENDIR packet is received. This eliminates the need for
the lock in the filelist() method and opens the path for eliminating
more locks.
Testing feasibility of creating the reader/writer when first receiving
the Open packet instead of waiting for the Get/Put.
This work is meant to make the Request locks easier to manage and
ultimately reducing/eliminating them as much as possible.
This fixes the concurrent access/locking race in both fileget and
fileput when accessing the saved readerAt/writerAt. It eliminates the
dedicated functions and moves the access/locking inline where they were
called to make it clearer.
Thanks to Marshall Brekka (@marshallbrekka) for the original bug report
and outline for the fix.
The initial Opendir packet is supposed to repond with an error status if
the directory wasn't found. It was just returning a handle without
checking, now it does a Stat on the path and only returns the handle if
the Stat is successful and it indicates it is a directory, otherwise it
returns an error.
When I moved the context below the close calls to fix the issue with
cancel interfering with file finalizing, I didn't take into account
the returns. The context cancel needed to be done regardless which this
does by moving it into a defer.
Don't return until FileLister returns io.EOF and 0 results. Fixes issue
where FileLister would return 0 results in one batch, but still have
more results to go.
Fixes#240
File reading/writeing might block on close until finished. If the
context's cancel is run first it can cancel the session before that
finishes. So the context's cancel should always be run last.
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().
packet_data structure was a leftover from previous code and it is no
longer necessary as the packets are available to use. I kept
packetData() to simplify grabing the data off the packets.
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).
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.
If a ListerAt implementation chooses to return fewer than MaxFileList
entries, the next offset it increased by MaxFileList in any case and so
we get unwanted results. Instead, increase the offset by the correct
number as returned by ListAt.
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
Cassondra Foesch
Nicola Murino
Fazlul Shahriar
Tommie Gannert
John Eikenberry
sig
John Eikenberry
Allan Feid
Mayank Agarwal
Martin Garton
sandreas