Fix race in log

[zeripath]

A race has been detected in #1441 relating to getting log levels.

This PR protects the GetLevel and GetStacktraceLevel calls with a mutex.

Signed-off-by: Andrew Thornton art27@cantab.net

[lunny]

To resolve the data race, the lock has to still be added into write loglevel in internalResetLevel.

[zeripath]

Both calls to that are already locked:

gitea/modules/log/event.go

Line 205 in a7fe58b

m.internalResetLevel()

gitea/modules/log/event.go

Line 393 in a7fe58b

return m.internalResetLevel()

[lunny]

The data race problem is some goroutines read MultiChannelledLog.level but others write MultiChannelledLog.level. It's not for multiple goroutines read MultiChannelledLog.level at the same time.

So I think you should add lock on MultiChannelledLog.level = xxx i.e.

gitea/modules/log/event.go

Line 375 in a7fe58b

m.level = NONE

And also for MultiChannelledLog.stacktraceLevel

[zeripath]

It's already locked.

gitea/modules/log/event.go

Lines 197 to 210 in a7fe58b

	func (m *MultiChannelledLog) DelLogger(name string) bool {
	m.mutex.Lock()
	logger, has := m.loggers[name]
	if !has {
	m.mutex.Unlock()
	return false
	}
	delete(m.loggers, name)
	m.internalResetLevel()
	m.mutex.Unlock()
	logger.Flush()
	logger.Close()
	return true
	}

gitea/modules/log/event.go

Lines 390 to 394 in a7fe58b

	func (m *MultiChannelledLog) ResetLevel() Level {
	m.mutex.Lock()
	defer m.mutex.Unlock()
	return m.internalResetLevel()
	}

[zeripath]

OK simple testing suggests that a RWMutex would be faster in the case of a lot of calls to GetLevel.

[lafriks]

There was no need to rename variable but otherwise lgtm

[zeripath]

Looks like this might be exposing another race in the test.

[zeripath]

Hmm... I think there might be no non-racey way to assert the batch_length so I'm going to remove that.

[zeripath]

make lgtm work