storage/filesystem: add R/W locking

This commit adds read/write locking for individual files, so that
concurrent requests (e.g. to read and write the same file) cannot
interfere with one another.

The locking is not very fine-grained at the moment, and can probably be
improved upon. But it does ensure consistency.
This commit is contained in:
Conrad Hoffmann 2024-11-07 12:14:45 +01:00
parent 7969df1a38
commit 6eeea854be
3 changed files with 88 additions and 2 deletions

View file

@ -10,6 +10,7 @@ import (
"path/filepath"
"regexp"
"strings"
"sync"
"github.com/rs/zerolog/log"
@ -20,9 +21,13 @@ import (
type filesystemBackend struct {
webdav.UserPrincipalBackend
path string
caldavPrefix string
carddavPrefix string
// maps file path to *sync.RWMutex
locks sync.Map
}
var (
@ -47,6 +52,36 @@ func NewFilesystem(fsPath, caldavPrefix, carddavPrefix string, userPrincipalBack
return backend, backend, nil
}
func (b *filesystemBackend) RLock(filename string) {
lock_, _ := b.locks.LoadOrStore(filename, &sync.RWMutex{})
lock := lock_.(*sync.RWMutex)
lock.RLock()
}
func (b *filesystemBackend) RUnlock(filename string) {
lock_, ok := b.locks.Load(filename)
if !ok {
panic("attempt to unlock non-existing lock")
}
lock := lock_.(*sync.RWMutex)
lock.RUnlock()
}
func (b *filesystemBackend) Lock(filename string) {
lock_, _ := b.locks.LoadOrStore(filename, &sync.RWMutex{})
lock := lock_.(*sync.RWMutex)
lock.Lock()
}
func (b *filesystemBackend) Unlock(filename string) {
lock_, ok := b.locks.Load(filename)
if !ok {
panic("attempt to unlock non-existing lock")
}
lock := lock_.(*sync.RWMutex)
lock.Unlock()
}
func ensureLocalDir(path string) error {
if _, err := os.Stat(path); os.IsNotExist(err) {
err = os.MkdirAll(path, 0755)

View file

@ -85,6 +85,9 @@ func (b *filesystemBackend) loadAllCalendarObjects(ctx context.Context, urlPath
return nil
}
b.RLock(filename)
defer b.RUnlock(filename)
cal, err := calendarFromFile(filename, propFilter)
if err != nil {
fmt.Printf("load calendar error for %s: %v\n", filename, err)
@ -138,7 +141,12 @@ func (b *filesystemBackend) createDefaultCalendar(ctx context.Context) (*caldav.
if err != nil {
return nil, fmt.Errorf("error creating default calendar: %s", err.Error())
}
err = os.WriteFile(path.Join(localPath, calendarFileName), blob, 0644)
filename := path.Join(localPath, calendarFileName)
b.Lock(filename)
defer b.Unlock(filename)
err = os.WriteFile(filename, blob, 0644)
if err != nil {
return nil, fmt.Errorf("error writing default calendar: %s", err.Error())
}
@ -167,6 +175,10 @@ func (b *filesystemBackend) ListCalendars(ctx context.Context) ([]caldav.Calenda
}
calPath := path.Join(filename, calendarFileName)
b.RLock(calPath)
defer b.RUnlock(calPath)
data, err := os.ReadFile(calPath)
if err != nil {
if os.IsNotExist(err) {
@ -209,6 +221,9 @@ func (b *filesystemBackend) GetCalendar(ctx context.Context, urlPath string) (*c
log.Debug().Str("path", localPath).Msg("loading calendar")
b.RLock(localPath)
defer b.RUnlock(localPath)
data, err := os.ReadFile(localPath)
if err != nil {
if os.IsNotExist(err) {
@ -237,6 +252,9 @@ func (b *filesystemBackend) GetCalendarObject(ctx context.Context, objPath strin
return nil, err
}
b.RLock(localPath)
defer b.RUnlock(localPath)
info, err := os.Stat(localPath)
if err != nil {
if errors.Is(err, fs.ErrNotExist) {
@ -320,6 +338,9 @@ func (b *filesystemBackend) PutCalendarObject(ctx context.Context, objPath strin
return nil, err
}
b.Lock(localPath)
defer b.Unlock(localPath)
flags := os.O_RDWR | os.O_CREATE | os.O_TRUNC
// TODO handle IfNoneMatch == ETag
if opts.IfNoneMatch.IsWildcard() {
@ -384,6 +405,10 @@ func (b *filesystemBackend) DeleteCalendarObject(ctx context.Context, path strin
if err != nil {
return err
}
b.Lock(localPath)
defer b.Unlock(localPath)
err = os.Remove(localPath)
if err != nil {
return err

View file

@ -69,6 +69,7 @@ func vcardPropFilter(card vcard.Card, props []string) vcard.Card {
}
func vcardFromFile(path string, propFilter []string) (vcard.Card, error) {
f, err := os.Open(path)
if err != nil {
return nil, err
@ -103,6 +104,10 @@ func (b *filesystemBackend) loadAllAddressObjects(ctx context.Context, urlPath s
return nil
}
// TODO use single file read for data & etag
b.RLock(filename)
defer b.RUnlock(filename)
card, err := vcardFromFile(filename, propFilter)
if err != nil {
return err
@ -141,7 +146,12 @@ func (b *filesystemBackend) writeAddressBook(ctx context.Context, ab *carddav.Ad
if err != nil {
return err
}
return os.WriteFile(path.Join(localPath, addressBookFileName), blob, 0644)
filename := path.Join(localPath, addressBookFileName)
b.Lock(filename)
defer b.Unlock(filename)
err = os.WriteFile(filename, blob, 0644)
if err != nil {
return fmt.Errorf("error writing address book: %s", err.Error())
}
@ -208,6 +218,9 @@ func (b *filesystemBackend) ListAddressBooks(ctx context.Context) ([]carddav.Add
}
abPath := path.Join(filename, addressBookFileName)
b.RLock(abPath)
defer b.RUnlock(abPath)
data, err := os.ReadFile(abPath)
if err != nil {
if os.IsNotExist(err) {
@ -250,6 +263,9 @@ func (b *filesystemBackend) GetAddressBook(ctx context.Context, urlPath string)
log.Debug().Str("path", localPath).Msg("loading addressbook")
b.RLock(localPath)
defer b.RUnlock(localPath)
data, err := os.ReadFile(localPath)
if err != nil {
if os.IsNotExist(err) {
@ -303,6 +319,9 @@ func (b *filesystemBackend) GetAddressObject(ctx context.Context, objPath string
return nil, err
}
b.RLock(localPath)
defer b.RUnlock(localPath)
info, err := os.Stat(localPath)
if err != nil {
if errors.Is(err, fs.ErrNotExist) {
@ -381,6 +400,9 @@ func (b *filesystemBackend) PutAddressObject(ctx context.Context, objPath string
return nil, err
}
b.Lock(localPath)
defer b.Unlock(localPath)
flags := os.O_RDWR | os.O_CREATE | os.O_TRUNC
// TODO handle IfNoneMatch == ETag
if opts.IfNoneMatch.IsWildcard() {
@ -444,6 +466,10 @@ func (b *filesystemBackend) DeleteAddressObject(ctx context.Context, path string
if err != nil {
return err
}
b.Lock(localPath)
defer b.Unlock(localPath)
err = os.Remove(localPath)
if err != nil {
return err