From cf67fe2cbc454a1f5a5d3290e1f8835bbfca94c0 Mon Sep 17 00:00:00 2001 From: Marty Schoch Date: Tue, 15 Dec 2015 16:39:38 -0500 Subject: [PATCH] fix major synchronization issue in the field_cache The field cache is expected to be the authority on which field names are identified by which identifier. This code was optimized for the most common case in which fields already exist. However, if we deterimine the field is missing with the read lock (shared), we incorrectly immediately proceed to create a new row with the write lock (exclusive). The problem is that multiple goroutines might have come to the same conclusion, and they all proceed to add rows. The two choices were to do the whole operation with the write lock, or recheck the value again with the write lock. We have chosen to repeat the check inside the write-lock, as this optimizes for what we believe to be the most common case, in which most fields will already exist. --- index/field_cache.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/index/field_cache.go b/index/field_cache.go index 206a55f6..535218b9 100644 --- a/index/field_cache.go +++ b/index/field_cache.go @@ -50,6 +50,11 @@ func (f *FieldCache) FieldNamed(field string, createIfMissing bool) (uint16, boo // trade read lock for write lock f.mutex.RUnlock() f.mutex.Lock() + // need to check again with write lock + if index, ok := f.fieldIndexes[field]; ok { + f.mutex.Unlock() + return index, true + } // assign next field id index := uint16(f.lastFieldIndex + 1) f.fieldIndexes[field] = index