Skip to content
This repository was archived by the owner on Jul 21, 2021. It is now read-only.

Conn.SetLogger() race condition #93

Open
youngkin opened this issue Nov 12, 2015 · 2 comments · May be fixed by #129
Open

Conn.SetLogger() race condition #93

youngkin opened this issue Nov 12, 2015 · 2 comments · May be fixed by #129

Comments

@youngkin
Copy link

I'd like to use Conn.SetLogger(Logger) to change the behavior of the default logger. go test -race reports a race with the call to this function and calls to c.logger.Printf(...) in conn.go (e.g., in the connect(...) function. I'm using SetLogger(...) as follows:

zkConn, zkConnEventChl, err := zk.Connect(zkServerList, timeoutMs)
nullLogger := nullZKLogger{}
zkConn.SetLogger(zk.Logger(nullLogger))

Am I using this capability incorrectly or is it a bug?

@ghost
Copy link

ghost commented Nov 12, 2015

I think the race is due to the fact that zk.Connect() spawns a goroutine that accesses zkConn.logger and calling zkConn.SetLogger() also touches that property. I don't know if this can really be a problem in practice, but you could solve it by passing an option to zk.Connect(). Namely, defining

func loggerOption(c *zk.Conn) {
    logger := nullZKLogger{}
    c.SetLogger(logger)
}

and then calling

zk.Connect(zkServerList, timeoutMs, loggerOption)

That way, the logger is set before the "loop" goroutine starts.

@nemith
Copy link
Contributor

nemith commented May 23, 2017

you can control this is a mutex lock as well.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants