search

goraft / raft

UNMAINTAINED: A Go implementation of the Raft distributed consensus protocol.
MIT License
2.43k stars 479 forks source link

Fix data race #123

Closed xiang90 closed 11 years ago

xiang90 commented 11 years ago

Fix data race. The major fix is to clone the requests before sending them to different raft server.

xiang90 commented 11 years ago

@benbjohnson This should fix #100.

erikstmartin commented 11 years ago

Awesome! Thanks Xiang, I was going to start peeking at it today :+1: :)

xiang90 commented 11 years ago

@benbjohnson Maybe we can merge this?

benbjohnson commented 11 years ago

@xiangli-cmu Sorry I didn't get to this sooner. Two points of feedback:

  1. I don't think we should expose setTerm() on an interface just for tests. Can you manually call s.(*server).mutex.Lock() in the tests instead and remove the setTerm() function? It's also strange because the initial-lowercase name will cause the function to not be exportable. I'm actually not sure how that would even work in Go.
  2. I'm still seeing a data race for event#returnValue. Can you wrap this in a getter/setter function with a lock? Here's the race condition report: https://gist.github.com/benbjohnson/763b7e55fa8f59244ba5
xiang90 commented 11 years ago

@benbjohnson For the second, I cannot understand it. I think it is already synced by the channel... I will merge this.