bronze1man / goStrongswanVici

a golang implement of strongswan vici plugin client.
MIT License
39 stars 34 forks source link

race condition between readThread() error handling and client error handling #19

Open lucwillems opened 7 years ago

lucwillems commented 7 years ago

hi,

will using the module using "-race" i got a race condition error on c.lastError. use case is as follow :

because of the error condition (charon down, no unix socket anymore), the readThread() will get a error in readSegment() and tries to set c.lastError.

but the connection is owned bij another go routine which runs the poll on Stat() which uses readResponse to get segments from the readerThread() using a responseChan channel

so readThread go routine does outMsg, err := readSegment(c.conn) if err != nil { c.lastError := err <=== write c.lastError return }

and user go routine does outMsg := c.readResponse() if c.lastError != nil { <==== read c.lastError return nil, c.lastError }

as stated in https://blog.golang.org/share-memory-by-communicating Do not communicate by sharing memory; instead, share memory by communicating.

we should use a chan error to communicate the error to the owner of the client connection and use that to set the c.LastError.

i'm currently preparing a patch to do this.

bronze1man commented 7 years ago

Just use sync.Mutex using chan in this case may make the program complex.