From 93c25c9a35141a1bb52966c5256ad7020e82ab79 Mon Sep 17 00:00:00 2001
From: Mura Li <typeless@users.noreply.github.com>
Date: Mon, 3 Apr 2017 10:22:26 +0800
Subject: [PATCH] Fix races in the log module by using syncmap (#1421)
---
modules/log/log.go | 54 ++--
vendor/golang.org/x/sync/LICENSE | 27 ++
vendor/golang.org/x/sync/PATENTS | 22 ++
vendor/golang.org/x/sync/syncmap/map.go | 372 ++++++++++++++++++++++++
vendor/vendor.json | 6 +
5 files changed, 455 insertions(+), 26 deletions(-)
create mode 100644 vendor/golang.org/x/sync/LICENSE
create mode 100644 vendor/golang.org/x/sync/PATENTS
create mode 100644 vendor/golang.org/x/sync/syncmap/map.go
diff --git a/modules/log/log.go b/modules/log/log.go
index 0b76002230..24ac3f652f 100644
--- a/modules/log/log.go
+++ b/modules/log/log.go
@@ -12,6 +12,8 @@ import (
"runtime"
"strings"
"sync"
+
+ "golang.org/x/sync/syncmap"
)
var (
@@ -42,10 +44,11 @@ func NewLogger(bufLen int64, mode, config string) {
// DelLogger removes loggers that are for the given mode
func DelLogger(mode string) error {
for _, l := range loggers {
- if _, ok := l.outputs[mode]; ok {
+ if _, ok := l.outputs.Load(mode); ok {
return l.DelLogger(mode)
}
}
+
Trace("Log adapter %s not found, no need to delete", mode)
return nil
}
@@ -177,16 +180,15 @@ type Logger struct {
lock sync.Mutex
level int
msg chan *logMsg
- outputs map[string]LoggerInterface
+ outputs syncmap.Map
quit chan bool
}
// newLogger initializes and returns a new logger.
func newLogger(buffer int64) *Logger {
l := &Logger{
- msg: make(chan *logMsg, buffer),
- outputs: make(map[string]LoggerInterface),
- quit: make(chan bool),
+ msg: make(chan *logMsg, buffer),
+ quit: make(chan bool),
}
go l.StartLogger()
return l
@@ -194,14 +196,12 @@ func newLogger(buffer int64) *Logger {
// SetLogger sets new logger instance with given logger adapter and config.
func (l *Logger) SetLogger(adapter string, config string) error {
- l.lock.Lock()
- defer l.lock.Unlock()
if log, ok := adapters[adapter]; ok {
lg := log()
if err := lg.Init(config); err != nil {
return err
}
- l.outputs[adapter] = lg
+ l.outputs.Store(adapter, lg)
l.adapter = adapter
} else {
panic("log: unknown adapter \"" + adapter + "\" (forgotten register?)")
@@ -211,11 +211,9 @@ func (l *Logger) SetLogger(adapter string, config string) error {
// DelLogger removes a logger adapter instance.
func (l *Logger) DelLogger(adapter string) error {
- l.lock.Lock()
- defer l.lock.Unlock()
- if lg, ok := l.outputs[adapter]; ok {
- lg.Destroy()
- delete(l.outputs, adapter)
+ if lg, ok := l.outputs.Load(adapter); ok {
+ lg.(LoggerInterface).Destroy()
+ l.outputs.Delete(adapter)
} else {
panic("log: unknown adapter \"" + adapter + "\" (forgotten register?)")
}
@@ -264,11 +262,12 @@ func (l *Logger) StartLogger() {
for {
select {
case bm := <-l.msg:
- for _, l := range l.outputs {
- if err := l.WriteMsg(bm.msg, bm.skip, bm.level); err != nil {
+ l.outputs.Range(func(k, v interface{}) bool {
+ if err := v.(LoggerInterface).WriteMsg(bm.msg, bm.skip, bm.level); err != nil {
fmt.Println("ERROR, unable to WriteMsg:", err)
}
- }
+ return true
+ })
case <-l.quit:
return
}
@@ -277,9 +276,10 @@ func (l *Logger) StartLogger() {
// Flush flushes all chan data.
func (l *Logger) Flush() {
- for _, l := range l.outputs {
- l.Flush()
- }
+ l.outputs.Range(func(k, v interface{}) bool {
+ v.(LoggerInterface).Flush()
+ return true
+ })
}
// Close closes logger, flush all chan data and destroy all adapter instances.
@@ -288,19 +288,21 @@ func (l *Logger) Close() {
for {
if len(l.msg) > 0 {
bm := <-l.msg
- for _, l := range l.outputs {
- if err := l.WriteMsg(bm.msg, bm.skip, bm.level); err != nil {
+ l.outputs.Range(func(k, v interface{}) bool {
+ if err := v.(LoggerInterface).WriteMsg(bm.msg, bm.skip, bm.level); err != nil {
fmt.Println("ERROR, unable to WriteMsg:", err)
}
- }
+ return true
+ })
} else {
break
}
}
- for _, l := range l.outputs {
- l.Flush()
- l.Destroy()
- }
+ l.outputs.Range(func(k, v interface{}) bool {
+ v.(LoggerInterface).Flush()
+ v.(LoggerInterface).Destroy()
+ return true
+ })
}
// Trace records trace log
diff --git a/vendor/golang.org/x/sync/LICENSE b/vendor/golang.org/x/sync/LICENSE
new file mode 100644
index 0000000000..6a66aea5ea
--- /dev/null
+++ b/vendor/golang.org/x/sync/LICENSE
@@ -0,0 +1,27 @@
+Copyright (c) 2009 The Go Authors. All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions are
+met:
+
+ * Redistributions of source code must retain the above copyright
+notice, this list of conditions and the following disclaimer.
+ * Redistributions in binary form must reproduce the above
+copyright notice, this list of conditions and the following disclaimer
+in the documentation and/or other materials provided with the
+distribution.
+ * Neither the name of Google Inc. nor the names of its
+contributors may be used to endorse or promote products derived from
+this software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
diff --git a/vendor/golang.org/x/sync/PATENTS b/vendor/golang.org/x/sync/PATENTS
new file mode 100644
index 0000000000..733099041f
--- /dev/null
+++ b/vendor/golang.org/x/sync/PATENTS
@@ -0,0 +1,22 @@
+Additional IP Rights Grant (Patents)
+
+"This implementation" means the copyrightable works distributed by
+Google as part of the Go project.
+
+Google hereby grants to You a perpetual, worldwide, non-exclusive,
+no-charge, royalty-free, irrevocable (except as stated in this section)
+patent license to make, have made, use, offer to sell, sell, import,
+transfer and otherwise run, modify and propagate the contents of this
+implementation of Go, where such license applies only to those patent
+claims, both currently owned or controlled by Google and acquired in
+the future, licensable by Google that are necessarily infringed by this
+implementation of Go. This grant does not include claims that would be
+infringed only as a consequence of further modification of this
+implementation. If you or your agent or exclusive licensee institute or
+order or agree to the institution of patent litigation against any
+entity (including a cross-claim or counterclaim in a lawsuit) alleging
+that this implementation of Go or any code incorporated within this
+implementation of Go constitutes direct or contributory patent
+infringement, or inducement of patent infringement, then any patent
+rights granted to you under this License for this implementation of Go
+shall terminate as of the date such litigation is filed.
diff --git a/vendor/golang.org/x/sync/syncmap/map.go b/vendor/golang.org/x/sync/syncmap/map.go
new file mode 100644
index 0000000000..80e15847ef
--- /dev/null
+++ b/vendor/golang.org/x/sync/syncmap/map.go
@@ -0,0 +1,372 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package syncmap provides a concurrent map implementation.
+// It is a prototype for a proposed addition to the sync package
+// in the standard library.
+// (https://golang.org/issue/18177)
+package syncmap
+
+import (
+ "sync"
+ "sync/atomic"
+ "unsafe"
+)
+
+// Map is a concurrent map with amortized-constant-time loads, stores, and deletes.
+// It is safe for multiple goroutines to call a Map's methods concurrently.
+//
+// The zero Map is valid and empty.
+//
+// A Map must not be copied after first use.
+type Map struct {
+ mu sync.Mutex
+
+ // read contains the portion of the map's contents that are safe for
+ // concurrent access (with or without mu held).
+ //
+ // The read field itself is always safe to load, but must only be stored with
+ // mu held.
+ //
+ // Entries stored in read may be updated concurrently without mu, but updating
+ // a previously-expunged entry requires that the entry be copied to the dirty
+ // map and unexpunged with mu held.
+ read atomic.Value // readOnly
+
+ // dirty contains the portion of the map's contents that require mu to be
+ // held. To ensure that the dirty map can be promoted to the read map quickly,
+ // it also includes all of the non-expunged entries in the read map.
+ //
+ // Expunged entries are not stored in the dirty map. An expunged entry in the
+ // clean map must be unexpunged and added to the dirty map before a new value
+ // can be stored to it.
+ //
+ // If the dirty map is nil, the next write to the map will initialize it by
+ // making a shallow copy of the clean map, omitting stale entries.
+ dirty map[interface{}]*entry
+
+ // misses counts the number of loads since the read map was last updated that
+ // needed to lock mu to determine whether the key was present.
+ //
+ // Once enough misses have occurred to cover the cost of copying the dirty
+ // map, the dirty map will be promoted to the read map (in the unamended
+ // state) and the next store to the map will make a new dirty copy.
+ misses int
+}
+
+// readOnly is an immutable struct stored atomically in the Map.read field.
+type readOnly struct {
+ m map[interface{}]*entry
+ amended bool // true if the dirty map contains some key not in m.
+}
+
+// expunged is an arbitrary pointer that marks entries which have been deleted
+// from the dirty map.
+var expunged = unsafe.Pointer(new(interface{}))
+
+// An entry is a slot in the map corresponding to a particular key.
+type entry struct {
+ // p points to the interface{} value stored for the entry.
+ //
+ // If p == nil, the entry has been deleted and m.dirty == nil.
+ //
+ // If p == expunged, the entry has been deleted, m.dirty != nil, and the entry
+ // is missing from m.dirty.
+ //
+ // Otherwise, the entry is valid and recorded in m.read.m[key] and, if m.dirty
+ // != nil, in m.dirty[key].
+ //
+ // An entry can be deleted by atomic replacement with nil: when m.dirty is
+ // next created, it will atomically replace nil with expunged and leave
+ // m.dirty[key] unset.
+ //
+ // An entry's associated value can be updated by atomic replacement, provided
+ // p != expunged. If p == expunged, an entry's associated value can be updated
+ // only after first setting m.dirty[key] = e so that lookups using the dirty
+ // map find the entry.
+ p unsafe.Pointer // *interface{}
+}
+
+func newEntry(i interface{}) *entry {
+ return &entry{p: unsafe.Pointer(&i)}
+}
+
+// Load returns the value stored in the map for a key, or nil if no
+// value is present.
+// The ok result indicates whether value was found in the map.
+func (m *Map) Load(key interface{}) (value interface{}, ok bool) {
+ read, _ := m.read.Load().(readOnly)
+ e, ok := read.m[key]
+ if !ok && read.amended {
+ m.mu.Lock()
+ // Avoid reporting a spurious miss if m.dirty got promoted while we were
+ // blocked on m.mu. (If further loads of the same key will not miss, it's
+ // not worth copying the dirty map for this key.)
+ read, _ = m.read.Load().(readOnly)
+ e, ok = read.m[key]
+ if !ok && read.amended {
+ e, ok = m.dirty[key]
+ // Regardless of whether the entry was present, record a miss: this key
+ // will take the slow path until the dirty map is promoted to the read
+ // map.
+ m.missLocked()
+ }
+ m.mu.Unlock()
+ }
+ if !ok {
+ return nil, false
+ }
+ return e.load()
+}
+
+func (e *entry) load() (value interface{}, ok bool) {
+ p := atomic.LoadPointer(&e.p)
+ if p == nil || p == expunged {
+ return nil, false
+ }
+ return *(*interface{})(p), true
+}
+
+// Store sets the value for a key.
+func (m *Map) Store(key, value interface{}) {
+ read, _ := m.read.Load().(readOnly)
+ if e, ok := read.m[key]; ok && e.tryStore(&value) {
+ return
+ }
+
+ m.mu.Lock()
+ read, _ = m.read.Load().(readOnly)
+ if e, ok := read.m[key]; ok {
+ if e.unexpungeLocked() {
+ // The entry was previously expunged, which implies that there is a
+ // non-nil dirty map and this entry is not in it.
+ m.dirty[key] = e
+ }
+ e.storeLocked(&value)
+ } else if e, ok := m.dirty[key]; ok {
+ e.storeLocked(&value)
+ } else {
+ if !read.amended {
+ // We're adding the first new key to the dirty map.
+ // Make sure it is allocated and mark the read-only map as incomplete.
+ m.dirtyLocked()
+ m.read.Store(readOnly{m: read.m, amended: true})
+ }
+ m.dirty[key] = newEntry(value)
+ }
+ m.mu.Unlock()
+}
+
+// tryStore stores a value if the entry has not been expunged.
+//
+// If the entry is expunged, tryStore returns false and leaves the entry
+// unchanged.
+func (e *entry) tryStore(i *interface{}) bool {
+ p := atomic.LoadPointer(&e.p)
+ if p == expunged {
+ return false
+ }
+ for {
+ if atomic.CompareAndSwapPointer(&e.p, p, unsafe.Pointer(i)) {
+ return true
+ }
+ p = atomic.LoadPointer(&e.p)
+ if p == expunged {
+ return false
+ }
+ }
+}
+
+// unexpungeLocked ensures that the entry is not marked as expunged.
+//
+// If the entry was previously expunged, it must be added to the dirty map
+// before m.mu is unlocked.
+func (e *entry) unexpungeLocked() (wasExpunged bool) {
+ return atomic.CompareAndSwapPointer(&e.p, expunged, nil)
+}
+
+// storeLocked unconditionally stores a value to the entry.
+//
+// The entry must be known not to be expunged.
+func (e *entry) storeLocked(i *interface{}) {
+ atomic.StorePointer(&e.p, unsafe.Pointer(i))
+}
+
+// LoadOrStore returns the existing value for the key if present.
+// Otherwise, it stores and returns the given value.
+// The loaded result is true if the value was loaded, false if stored.
+func (m *Map) LoadOrStore(key, value interface{}) (actual interface{}, loaded bool) {
+ // Avoid locking if it's a clean hit.
+ read, _ := m.read.Load().(readOnly)
+ if e, ok := read.m[key]; ok {
+ actual, loaded, ok := e.tryLoadOrStore(value)
+ if ok {
+ return actual, loaded
+ }
+ }
+
+ m.mu.Lock()
+ read, _ = m.read.Load().(readOnly)
+ if e, ok := read.m[key]; ok {
+ if e.unexpungeLocked() {
+ m.dirty[key] = e
+ }
+ actual, loaded, _ = e.tryLoadOrStore(value)
+ } else if e, ok := m.dirty[key]; ok {
+ actual, loaded, _ = e.tryLoadOrStore(value)
+ m.missLocked()
+ } else {
+ if !read.amended {
+ // We're adding the first new key to the dirty map.
+ // Make sure it is allocated and mark the read-only map as incomplete.
+ m.dirtyLocked()
+ m.read.Store(readOnly{m: read.m, amended: true})
+ }
+ m.dirty[key] = newEntry(value)
+ actual, loaded = value, false
+ }
+ m.mu.Unlock()
+
+ return actual, loaded
+}
+
+// tryLoadOrStore atomically loads or stores a value if the entry is not
+// expunged.
+//
+// If the entry is expunged, tryLoadOrStore leaves the entry unchanged and
+// returns with ok==false.
+func (e *entry) tryLoadOrStore(i interface{}) (actual interface{}, loaded, ok bool) {
+ p := atomic.LoadPointer(&e.p)
+ if p == expunged {
+ return nil, false, false
+ }
+ if p != nil {
+ return *(*interface{})(p), true, true
+ }
+
+ // Copy the interface after the first load to make this method more amenable
+ // to escape analysis: if we hit the "load" path or the entry is expunged, we
+ // shouldn't bother heap-allocating.
+ ic := i
+ for {
+ if atomic.CompareAndSwapPointer(&e.p, nil, unsafe.Pointer(&ic)) {
+ return i, false, true
+ }
+ p = atomic.LoadPointer(&e.p)
+ if p == expunged {
+ return nil, false, false
+ }
+ if p != nil {
+ return *(*interface{})(p), true, true
+ }
+ }
+}
+
+// Delete deletes the value for a key.
+func (m *Map) Delete(key interface{}) {
+ read, _ := m.read.Load().(readOnly)
+ e, ok := read.m[key]
+ if !ok && read.amended {
+ m.mu.Lock()
+ read, _ = m.read.Load().(readOnly)
+ e, ok = read.m[key]
+ if !ok && read.amended {
+ delete(m.dirty, key)
+ }
+ m.mu.Unlock()
+ }
+ if ok {
+ e.delete()
+ }
+}
+
+func (e *entry) delete() (hadValue bool) {
+ for {
+ p := atomic.LoadPointer(&e.p)
+ if p == nil || p == expunged {
+ return false
+ }
+ if atomic.CompareAndSwapPointer(&e.p, p, nil) {
+ return true
+ }
+ }
+}
+
+// Range calls f sequentially for each key and value present in the map.
+// If f returns false, range stops the iteration.
+//
+// Range does not necessarily correspond to any consistent snapshot of the Map's
+// contents: no key will be visited more than once, but if the value for any key
+// is stored or deleted concurrently, Range may reflect any mapping for that key
+// from any point during the Range call.
+//
+// Range may be O(N) with the number of elements in the map even if f returns
+// false after a constant number of calls.
+func (m *Map) Range(f func(key, value interface{}) bool) {
+ // We need to be able to iterate over all of the keys that were already
+ // present at the start of the call to Range.
+ // If read.amended is false, then read.m satisfies that property without
+ // requiring us to hold m.mu for a long time.
+ read, _ := m.read.Load().(readOnly)
+ if read.amended {
+ // m.dirty contains keys not in read.m. Fortunately, Range is already O(N)
+ // (assuming the caller does not break out early), so a call to Range
+ // amortizes an entire copy of the map: we can promote the dirty copy
+ // immediately!
+ m.mu.Lock()
+ read, _ = m.read.Load().(readOnly)
+ if read.amended {
+ read = readOnly{m: m.dirty}
+ m.read.Store(read)
+ m.dirty = nil
+ m.misses = 0
+ }
+ m.mu.Unlock()
+ }
+
+ for k, e := range read.m {
+ v, ok := e.load()
+ if !ok {
+ continue
+ }
+ if !f(k, v) {
+ break
+ }
+ }
+}
+
+func (m *Map) missLocked() {
+ m.misses++
+ if m.misses < len(m.dirty) {
+ return
+ }
+ m.read.Store(readOnly{m: m.dirty})
+ m.dirty = nil
+ m.misses = 0
+}
+
+func (m *Map) dirtyLocked() {
+ if m.dirty != nil {
+ return
+ }
+
+ read, _ := m.read.Load().(readOnly)
+ m.dirty = make(map[interface{}]*entry, len(read.m))
+ for k, e := range read.m {
+ if !e.tryExpungeLocked() {
+ m.dirty[k] = e
+ }
+ }
+}
+
+func (e *entry) tryExpungeLocked() (isExpunged bool) {
+ p := atomic.LoadPointer(&e.p)
+ for p == nil {
+ if atomic.CompareAndSwapPointer(&e.p, nil, expunged) {
+ return true
+ }
+ p = atomic.LoadPointer(&e.p)
+ }
+ return p == expunged
+}
diff --git a/vendor/vendor.json b/vendor/vendor.json
index 4b24848ea7..d5ad949feb 100644
--- a/vendor/vendor.json
+++ b/vendor/vendor.json
@@ -1266,6 +1266,12 @@
"revision": "569280fa63be4e201b975e5411e30a92178f0118",
"revisionTime": "2016-11-03T00:14:07Z"
},
+ {
+ "checksumSHA1": "4TEYFKrAUuwBMqExjQBsnf/CgjQ=",
+ "path": "golang.org/x/sync/syncmap",
+ "revision": "5a06fca2c336a4b2b2fcb45702e8c47621b2aa2c",
+ "revisionTime": "2017-03-17T17:13:11Z"
+ },
{
"checksumSHA1": "8fD/im5Kwvy3JgmxulDTambmE8w=",
"path": "golang.org/x/sys/unix",