From a193146c91dea4b6cf30f355e99fb85f73d20f07 Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Thu, 11 Aug 2016 22:11:21 -0400 Subject: [PATCH] Fix deadlock on rewrite of opHistory when history limit is reached (#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. --- history.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/history.go b/history.go index 900a26e..042f2f5 100644 --- a/history.go +++ b/history.go @@ -5,8 +5,8 @@ import ( "container/list" "fmt" "os" - "sync" "strings" + "sync" ) type hisItem struct { @@ -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) { @@ -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) @@ -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 }