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
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.
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.
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
Updates #192
Move handle() onto *Request, this has flow on effects to the interface
types declared in request-interface.go, the examples and the tests.
The decision to address #192 started with advice from gopl.io, but I
think has uncovered several bugs as many of the request methods operate
on copies of data stored in the RequestServer's cache. I think this is
unintentional, but may point to some correctness issues, see #193.
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
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.
Cassondra Foesch
Nicola Murino
John Eikenberry
sandreas
Dave Cheney