Skip to content

Commit

Permalink
request server: call Close() on ListerAt if it implements io.Closer
Browse files Browse the repository at this point in the history
The ListerAt is stored in the Request state and reused across requests.
Some implementations don't store the entire []os.FileInfo buffer in the
ListerAt implementation but instead return an open file and get/return
[]os.FileInfo on request. For these implementation calling Close is
required
  • Loading branch information
drakkan committed Feb 14, 2024
1 parent 46d90e3 commit fbb0b8b
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 2 deletions.
2 changes: 2 additions & 0 deletions request-interfaces.go
Expand Up @@ -144,6 +144,8 @@ type NameLookupFileLister interface {
//
// If a populated entry implements [FileInfoExtendedData], extended attributes will also be returned to the client.
//
// The request server code will call Close() on ListerAt if an io.Closer type assertion succeeds.
//
// Note in cases of an error, the error text will be sent to the client.
type ListerAt interface {
ListAt([]os.FileInfo, int64) (int, error)
Expand Down
31 changes: 31 additions & 0 deletions request-server_test.go
Expand Up @@ -793,6 +793,37 @@ func TestRequestReaddir(t *testing.T) {
checkRequestServerAllocator(t, p)
}

type testListerAtCloser struct {
isClosed bool
}

func (l *testListerAtCloser) ListAt([]os.FileInfo, int64) (int, error) {
return 0, io.EOF
}

func (l *testListerAtCloser) Close() error {
l.isClosed = true
return nil
}

func TestRequestServerListerAtCloser(t *testing.T) {
p := clientRequestServerPair(t)
defer p.Close()

handle, err := p.cli.opendir(context.Background(), "/")
require.NoError(t, err)
require.Len(t, p.svr.openRequests, 1)
req, ok := p.svr.getRequest(handle)
require.True(t, ok)
listerAt := &testListerAtCloser{}
req.setListerAt(listerAt)
assert.NotNil(t, req.state.getListerAt())
err = p.cli.close(handle)
assert.NoError(t, err)
require.Len(t, p.svr.openRequests, 0)
assert.True(t, listerAt.isClosed)
}

func TestRequestStatVFS(t *testing.T) {
if runtime.GOOS != "linux" && runtime.GOOS != "darwin" {
t.Skip("StatVFS is implemented on linux and darwin")
Expand Down
20 changes: 18 additions & 2 deletions request.go
Expand Up @@ -121,6 +121,22 @@ func (s *state) getListerAt() ListerAt {
return s.listerAt
}

func (s *state) closeListerAt() error {
s.mu.Lock()
defer s.mu.Unlock()

var err error

if s.listerAt != nil {
if c, ok := s.listerAt.(io.Closer); ok {
err = c.Close()
}
s.listerAt = nil
}

return err
}

// Request contains the data and state for the incoming service request.
type Request struct {
// Get, Put, Setstat, Stat, Rename, Remove
Expand Down Expand Up @@ -230,9 +246,9 @@ func (r *Request) close() error {
}
}()

rd, wr, rw := r.getAllReaderWriters()
err := r.state.closeListerAt()

var err error
rd, wr, rw := r.getAllReaderWriters()

// Close errors on a Writer are far more likely to be the important one.
// As they can be information that there was a loss of data.
Expand Down

0 comments on commit fbb0b8b

Please sign in to comment.