Fix data races in MemMapFs

Also simplifiy the lock constructs.

Fixes #44
Fixes #45
This commit is contained in:
Bjørn Erik Pedersen 2015-12-18 19:20:45 +01:00
parent 36b075cbbf
commit 75b0bd216a
1 changed files with 34 additions and 49 deletions

View File

@ -29,47 +29,32 @@ import (
var mux = &sync.Mutex{} var mux = &sync.Mutex{}
type MemMapFs struct { type MemMapFs struct {
sync.RWMutex
data map[string]File data map[string]File
mutex *sync.RWMutex init sync.Once
} }
func (m *MemMapFs) lock() { var memfsInit sync.Once
mx := m.getMutex()
mx.Lock()
}
func (m *MemMapFs) unlock() { m.getMutex().Unlock() }
func (m *MemMapFs) rlock() { m.getMutex().RLock() }
func (m *MemMapFs) runlock() { m.getMutex().RUnlock() }
func (m *MemMapFs) getData() map[string]File { func (m *MemMapFs) getData() map[string]File {
if m.data == nil { m.init.Do(func() {
m.data = make(map[string]File) m.data = make(map[string]File)
// Root should always exist, right? // Root should always exist, right?
// TODO: what about windows? // TODO: what about windows?
m.data[FilePathSeparator] = mem.CreateDir(FilePathSeparator) m.data[FilePathSeparator] = mem.CreateDir(FilePathSeparator)
} })
return m.data return m.data
} }
func (m *MemMapFs) getMutex() *sync.RWMutex {
mux.Lock()
if m.mutex == nil {
m.mutex = &sync.RWMutex{}
}
mux.Unlock()
return m.mutex
}
func (MemMapFs) Name() string { return "MemMapFS" } func (MemMapFs) Name() string { return "MemMapFS" }
func (m *MemMapFs) Create(name string) (File, error) { func (m *MemMapFs) Create(name string) (File, error) {
name = normalizePath(name) name = normalizePath(name)
m.lock() m.Lock()
file := mem.CreateFile(name) file := mem.CreateFile(name)
m.getData()[name] = file m.getData()[name] = file
m.registerWithParent(file) m.registerWithParent(file)
m.unlock() m.Unlock()
return file, nil return file, nil
} }
@ -152,9 +137,9 @@ func (m *MemMapFs) lockfreeMkdir(name string, perm os.FileMode) error {
func (m *MemMapFs) Mkdir(name string, perm os.FileMode) error { func (m *MemMapFs) Mkdir(name string, perm os.FileMode) error {
name = normalizePath(name) name = normalizePath(name)
m.rlock() m.RLock()
x, ok := m.getData()[name] x, ok := m.getData()[name]
m.runlock() m.RUnlock()
if ok { if ok {
// Only return ErrFileExists if it's a file, not a directory. // Only return ErrFileExists if it's a file, not a directory.
i, err := x.Stat() i, err := x.Stat()
@ -165,11 +150,11 @@ func (m *MemMapFs) Mkdir(name string, perm os.FileMode) error {
return err return err
} }
} else { } else {
m.lock() m.Lock()
item := mem.CreateDir(name) item := mem.CreateDir(name)
m.getData()[name] = item m.getData()[name] = item
m.registerWithParent(item) m.registerWithParent(item)
m.unlock() m.Unlock()
} }
return nil return nil
} }
@ -195,14 +180,14 @@ func normalizePath(path string) string {
func (m *MemMapFs) Open(name string) (File, error) { func (m *MemMapFs) Open(name string) (File, error) {
name = normalizePath(name) name = normalizePath(name)
m.rlock() m.RLock()
f, ok := m.getData()[name] f, ok := m.getData()[name]
ff, ok := f.(*mem.File) ff, ok := f.(*mem.File)
if ok { if ok {
ff.Open() ff.Open()
} }
m.runlock() m.RUnlock()
if ok { if ok {
return f, nil return f, nil
@ -253,8 +238,8 @@ func (m *MemMapFs) OpenFile(name string, flag int, perm os.FileMode) (File, erro
func (m *MemMapFs) Remove(name string) error { func (m *MemMapFs) Remove(name string) error {
name = normalizePath(name) name = normalizePath(name)
m.lock() m.Lock()
defer m.unlock() defer m.Unlock()
if _, ok := m.getData()[name]; ok { if _, ok := m.getData()[name]; ok {
m.unRegisterWithParent(name) m.unRegisterWithParent(name)
@ -267,20 +252,20 @@ func (m *MemMapFs) Remove(name string) error {
func (m *MemMapFs) RemoveAll(path string) error { func (m *MemMapFs) RemoveAll(path string) error {
path = normalizePath(path) path = normalizePath(path)
m.lock() m.Lock()
m.unRegisterWithParent(path) m.unRegisterWithParent(path)
m.unlock() m.Unlock()
m.rlock() m.RLock()
defer m.runlock() defer m.RUnlock()
for p, _ := range m.getData() { for p, _ := range m.getData() {
if strings.HasPrefix(p, path) { if strings.HasPrefix(p, path) {
m.runlock() m.RUnlock()
m.lock() m.Lock()
delete(m.getData(), p) delete(m.getData(), p)
m.unlock() m.Unlock()
m.rlock() m.RLock()
} }
} }
return nil return nil
@ -294,20 +279,20 @@ func (m *MemMapFs) Rename(oldname, newname string) error {
return nil return nil
} }
m.rlock() m.RLock()
defer m.runlock() defer m.RUnlock()
if _, ok := m.getData()[oldname]; ok { if _, ok := m.getData()[oldname]; ok {
if _, ok := m.getData()[newname]; !ok { if _, ok := m.getData()[newname]; !ok {
m.runlock() m.RUnlock()
m.lock() m.Lock()
m.unRegisterWithParent(oldname) m.unRegisterWithParent(oldname)
file := m.getData()[oldname].(*mem.File) file := m.getData()[oldname].(*mem.File)
delete(m.getData(), oldname) delete(m.getData(), oldname)
mem.ChangeFileName(file, newname) mem.ChangeFileName(file, newname)
m.getData()[newname] = file m.getData()[newname] = file
m.registerWithParent(file) m.registerWithParent(file)
m.unlock() m.Unlock()
m.rlock() m.RLock()
} else { } else {
return ErrDestinationExists return ErrDestinationExists
} }
@ -335,9 +320,9 @@ func (m *MemMapFs) Chmod(name string, mode os.FileMode) error {
ff, ok := f.(*mem.File) ff, ok := f.(*mem.File)
if ok { if ok {
m.lock() m.Lock()
mem.SetMode(ff, mode) mem.SetMode(ff, mode)
m.unlock() m.Unlock()
} else { } else {
return errors.New("Unable to Chmod Memory File") return errors.New("Unable to Chmod Memory File")
} }
@ -353,9 +338,9 @@ func (m *MemMapFs) Chtimes(name string, atime time.Time, mtime time.Time) error
ff, ok := f.(*mem.File) ff, ok := f.(*mem.File)
if ok { if ok {
m.lock() m.Lock()
mem.SetModTime(ff, mtime) mem.SetModTime(ff, mtime)
m.unlock() m.Unlock()
} else { } else {
return errors.New("Unable to Chtime Memory File") return errors.New("Unable to Chtime Memory File")
} }