fix data race on set level and formatter

[klizhentas]

fixes #295

[ash2k]

You may avoid some overhead by not using defer as it is not strictly necessary here - reading a field can not panic and logger is not nil because it was accessed one line above. Same for the level() method.

[aarongreenlee]

If you could make the small adjustment to remove the defer and ping me, I'd be happy to merge this pull request.

Thanks for contributing this improvement!

[klizhentas]

@aarongreenlee I've addressed the code review comment

[klizhentas]

@aarongreenlee ping

[aarongreenlee]

Thanks, @klizhentas. This all looks good to me. Can you please resolve these minor conflicts? I'll merge shortly thereafter.

Thanks for this contribution.

[johanbrandhorst]

@klizhentas pinging to have merge conflicts resolved. Good work!

[klizhentas]

I don't think it's actually possible to have my changeset to coexist with the master mostly because of this chunk of code that turns off locking:

https://github.com/Sirupsen/logrus/blob/master/logger.go#L53

https://github.com/Sirupsen/logrus/blob/master/logger.go#L306

Moreover I think the new MutexWrap introduces one more race condition on it's own - the disabled variable read/writes are not atomic and should be actually atomic operations. In addition to that in the new codebase there's theoretically could be a condition when you:

Lock()
SetNoLock()
Unlock()

getting a deadlock. Everyones ideas on getting this altogether are appreciated.

[stevvooe]

Is there an equivalent (*Logger).SetLevel? It seems odd that this method would be at the package level but use a struct bound lock without offering the method on the struct itself.

[DmitriyMV]

I think it's partially fixed by #512, but Formatter problem remains. The mutex will prevent that but it will also introduce performance penalities in situations where the same log is used by several goroutines in parallel. Additional info here: golang/go#17973 TLDR - it doesn't scale well.

However, I'm still unconvinced that we really should bother about that in the first place, given the fact the most of the loggers and hooks are IO bound. Any thoughts on the matter? Synthetic benchmarks do not really show the realistic scenarios.