Skip to content

Commit

Permalink
Fix deadlock on rewrite of opHistory when history limit is reached (c…
Browse files Browse the repository at this point in the history
…hzyer#69)

It is possible for `opHistory.Rewrite` to be called from
`opHistory.historyUpdatePath`. This is problematic, because both methods
grab a lock, and Mutexes in go are not reentrant. This change pulls out
the logic in Rewrite into `opHistory.rewriteLocked`, and retains the
public facing method.
  • Loading branch information
nvanbenschoten authored and chzyer committed Aug 12, 2016
1 parent c144c8d commit a193146
Showing 1 changed file with 7 additions and 3 deletions.
10 changes: 7 additions & 3 deletions history.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import (
"container/list"
"fmt"
"os"
"sync"
"strings"
"sync"
)

type hisItem struct {
Expand All @@ -26,7 +26,7 @@ type opHistory struct {
historyVer int64
current *list.Element
fd *os.File
fdLock sync.Mutex
fdLock sync.Mutex
}

func newOpHistory(cfg *Config) (o *opHistory) {
Expand Down Expand Up @@ -85,7 +85,7 @@ func (o *opHistory) historyUpdatePath(path string) {
o.Compact()
}
if total > o.cfg.HistoryLimit {
o.Rewrite()
o.rewriteLocked()
}
o.historyVer++
o.Push(nil)
Expand All @@ -101,6 +101,10 @@ func (o *opHistory) Compact() {
func (o *opHistory) Rewrite() {
o.fdLock.Lock()
defer o.fdLock.Unlock()
o.rewriteLocked()
}

func (o *opHistory) rewriteLocked() {
if o.cfg.HistoryFile == "" {
return
}
Expand Down

0 comments on commit a193146

Please sign in to comment.