gansteed created THRIFT-4552: -------------------------------- Summary: why acquire a lock in TSimpleServer implementation for Go? Key: THRIFT-4552 URL: https://issues.apache.org/jira/browse/THRIFT-4552 Project: Thrift Issue Type: Improvement Affects Versions: 0.11.0 Reporter: gansteed
I've sent a email to groups, but I think maybe here will be better? I'm using Thrift and I'm reading thrift implementation for Go, I found code in `TSimpleServer.AcceptLoop` like this: ```go func (p *TSimpleServer) AcceptLoop() error { for { client, err := p.serverTransport.Accept() p.mu.Lock() if atomic.LoadInt32(&p.closed) != 0 { return nil } if err != nil { return err } if client != nil { p.wg.Add(1) go func() { defer p.wg.Done() if err := p.processRequests(client); err != nil { log.Println("error processing request:", err) } }() } p.mu.Unlock() } } ``` every time it accept a request,it: 1. read if protocol had been closed, this step is atomic, it does not need a lock. 2. p.wg.Add(1) to accumulate a goroutine? this step is atomic, too, it does not need a lock 3. after processor processed the request, it do p.wg.Done(), it's atomic, too, and it does not need a lock. by the way, it seems that `p.wg.Done()` do not need to put in defer? just put it after p.processRequests(client)? so is there any particular to do it in this way?if not, I would like to submit a PR to reduce unneccessary performance overhead in TSimpleServer implementation. -- This message was sent by Atlassian JIRA (v7.6.3#76005)