Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Thread safety #104

Open
yacovm opened this issue Sep 28, 2016 · 0 comments
Open

Thread safety #104

yacovm opened this issue Sep 28, 2016 · 0 comments

Comments

@yacovm
Copy link

yacovm commented Sep 28, 2016 •
edited
Loading

Hi.

Logger.log() calls moduleLeveled.Log() which
calls moduleLeveled.GetLevel(), which does a map read.
Now, var defaultBackend LeveledBackend is a global field.
If someone calls concurrently logging.SetLevel(), a map write takes place in moduleLeveled.SetLevel() which results in a panic of "fatal error: concurrent map read and map write".

Is it possible to protect the backend via a read-write lock?

Attached a stack trace that caused a panic:

15:12:00 runtime.throw(0xd52020, 0x21)

15:12:00 /opt/go/go1.6.linux.amd64/src/runtime/panic.go:530 +0x90 fp=0xc8204cbba8 sp=0xc8204cbb90

15:12:00 runtime.mapaccess2_faststr(0xa7da00, 0xc8201b8690, 0xca16c0, 0x3, 0x0, 0xe2c800000036)

15:12:00 /opt/go/go1.6.linux.amd64/src/runtime/hashmap_fast.go:307 +0x5b fp=0xc8204cbc08 sp=0xc8204cbba8

15:12:00 github.com/hyperledger/fabric/vendor/github.com/op/go-logging.(*moduleLeveled).Log(0xc82015eb80, 0x4, 0x2, 0xc82035ce00, 0x0, 0x0)

15:12:00 /w/workspace/fabric-verify-x86_64/gopath/src/github.com/hyperledger/fabric/vendor/github.com/op/go-logging/level.go:113 +0xa7 fp=0xc8204cbca0 sp=0xc8204cbc08

15:12:00 github.com/hyperledger/fabric/vendor/github.com/op/go-logging.(*Logger).log(0xc8201b87e0, 0x4, 0x0, 0xc82031e980, 0x1, 0x1)

15:12:00 /w/workspace/fabric-verify-x86_64/gopath/src/github.com/hyperledger/fabric/vendor/github.com/op/go-logging/logger.go:170 +0x1f3 fp=0xc8204cbd10 sp=0xc8204cbca0

15:12:00 github.com/hyperledger/fabric/vendor/github.com/op/go-logging.(*Logger).Info(0xc8201b87e0, 0xc82031e980, 0x1, 0x1)

This problem manifests when running multiple go tests concurrently, when launching multiple instances of objects that use your package at the same time.

Update: I opened a PR
#105
yacovm added a commit to yacovm/go-logging that referenced this issue Sep 30, 2016
@yacovm
Thread safety fix in logging and SetLevel
b042c5e 
op#104

When a logging is called concurrently with SetLevel, a panic occurs:
fatal error: concurrent map read and map write

goroutine 20 [running]:
runtime.throw(0x5f7cc0, 0x21)
	/home/yacovm/OBC/go/src/runtime/panic.go:530 +0x90 fp=0xc820038d58 sp=0xc820038d40
runtime.mapaccess2_faststr(0x551020, 0xc820078780, 0x5c9cc8, 0x4, 0x10, 0x551820)
	/home/yacovm/OBC/go/src/runtime/hashmap_fast.go:307 +0x5b fp=0xc820038db8 sp=0xc820038d58
_/home/yacovm/go-logging/go-logging.(*moduleLeveled).IsEnabledFor(0xc8200981e0, 0x4, 0x5c9cc8, 0x4, 0x0)
	/home/yacovm/go-logging/go-logging/level.go:110 +0x62 fp=0xc820038e18 sp=0xc820038db8
_/home/yacovm/go-logging/go-logging.(*Logger).IsEnabledFor(0xc8200788a0, 0x4, 0x0)
	/home/yacovm/go-logging/go-logging/logger.go:140 +0x4d fp=0xc820038e48 sp=0xc820038e18
_/home/yacovm/go-logging/go-logging.(*Logger).log(0xc8200788a0, 0x4, 0x0, 0xc8200d9b30, 0x1, 0x1)
	/home/yacovm/go-logging/go-logging/logger.go:144 +0x2f fp=0xc820038eb8 sp=0xc820038e48
_/home/yacovm/go-logging/go-logging.(*Logger).Info(0xc8200788a0, 0xc8200d9b30, 0x1, 0x1)
	/home/yacovm/go-logging/go-logging/logger.go:239 +0x51 fp=0xc820038ef0 sp=0xc820038eb8
_/home/yacovm/go-logging/go-logging.TestConcurrency(0xc8200a0120)
	/home/yacovm/go-logging/go-logging/logger_test.go:73 +0x11f fp=0xc820038f68 sp=0xc820038ef0
testing.tRunner(0xc8200a0120, 0x6b8468)

I added a RWLock to protect the map and added a regression test that many times
fails without the locks
@yacovm yacovm mentioned this issue Sep 30, 2016
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@yacovm