From 75b0bd216a4b4f527dae351073b259fef4f00f81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Fri, 18 Dec 2015 19:20:45 +0100 Subject: [PATCH] Fix data races in MemMapFs Also simplifiy the lock constructs. Fixes #44 Fixes #45 --- memmap.go | 83 +++++++++++++++++++++++-------------------------------- 1 file changed, 34 insertions(+), 49 deletions(-) diff --git a/memmap.go b/memmap.go index 3a5a918..0b4edf4 100644 --- a/memmap.go +++ b/memmap.go @@ -29,47 +29,32 @@ import ( var mux = &sync.Mutex{} type MemMapFs struct { - data map[string]File - mutex *sync.RWMutex + sync.RWMutex + data map[string]File + init sync.Once } -func (m *MemMapFs) lock() { - 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() } +var memfsInit sync.Once func (m *MemMapFs) getData() map[string]File { - if m.data == nil { + m.init.Do(func() { m.data = make(map[string]File) - // Root should always exist, right? // TODO: what about windows? m.data[FilePathSeparator] = mem.CreateDir(FilePathSeparator) - } + }) 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 (m *MemMapFs) Create(name string) (File, error) { name = normalizePath(name) - m.lock() + m.Lock() file := mem.CreateFile(name) m.getData()[name] = file m.registerWithParent(file) - m.unlock() + m.Unlock() 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 { name = normalizePath(name) - m.rlock() + m.RLock() x, ok := m.getData()[name] - m.runlock() + m.RUnlock() if ok { // Only return ErrFileExists if it's a file, not a directory. i, err := x.Stat() @@ -165,11 +150,11 @@ func (m *MemMapFs) Mkdir(name string, perm os.FileMode) error { return err } } else { - m.lock() + m.Lock() item := mem.CreateDir(name) m.getData()[name] = item m.registerWithParent(item) - m.unlock() + m.Unlock() } return nil } @@ -195,14 +180,14 @@ func normalizePath(path string) string { func (m *MemMapFs) Open(name string) (File, error) { name = normalizePath(name) - m.rlock() + m.RLock() f, ok := m.getData()[name] ff, ok := f.(*mem.File) if ok { ff.Open() } - m.runlock() + m.RUnlock() if ok { 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 { name = normalizePath(name) - m.lock() - defer m.unlock() + m.Lock() + defer m.Unlock() if _, ok := m.getData()[name]; ok { m.unRegisterWithParent(name) @@ -267,20 +252,20 @@ func (m *MemMapFs) Remove(name string) error { func (m *MemMapFs) RemoveAll(path string) error { path = normalizePath(path) - m.lock() + m.Lock() m.unRegisterWithParent(path) - m.unlock() + m.Unlock() - m.rlock() - defer m.runlock() + m.RLock() + defer m.RUnlock() for p, _ := range m.getData() { if strings.HasPrefix(p, path) { - m.runlock() - m.lock() + m.RUnlock() + m.Lock() delete(m.getData(), p) - m.unlock() - m.rlock() + m.Unlock() + m.RLock() } } return nil @@ -294,20 +279,20 @@ func (m *MemMapFs) Rename(oldname, newname string) error { return nil } - m.rlock() - defer m.runlock() + m.RLock() + defer m.RUnlock() if _, ok := m.getData()[oldname]; ok { if _, ok := m.getData()[newname]; !ok { - m.runlock() - m.lock() + m.RUnlock() + m.Lock() m.unRegisterWithParent(oldname) file := m.getData()[oldname].(*mem.File) delete(m.getData(), oldname) mem.ChangeFileName(file, newname) m.getData()[newname] = file m.registerWithParent(file) - m.unlock() - m.rlock() + m.Unlock() + m.RLock() } else { return ErrDestinationExists } @@ -335,9 +320,9 @@ func (m *MemMapFs) Chmod(name string, mode os.FileMode) error { ff, ok := f.(*mem.File) if ok { - m.lock() + m.Lock() mem.SetMode(ff, mode) - m.unlock() + m.Unlock() } else { 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) if ok { - m.lock() + m.Lock() mem.SetModTime(ff, mtime) - m.unlock() + m.Unlock() } else { return errors.New("Unable to Chtime Memory File") }