gansteed created THRIFT-4552:

             Summary: why acquire a lock in TSimpleServer implementation for Go?
                 Key: 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:
func (p *TSimpleServer) AcceptLoop() error {
        for {
                client, err := p.serverTransport.Accept()
                if atomic.LoadInt32(&p.closed) != 0 {
                        return nil
                if err != nil {
                        return err
                if client != nil {
                        go func() {
                                defer p.wg.Done()
                                if err := p.processRequests(client); err != nil 
                                        log.Println("error processing 
request:", err)
every time it accept a request,it:
1. read if protocol had been closed, this step is atomic, it does not need a 
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 

This message was sent by Atlassian JIRA

Reply via email to