You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by gansteed <ga...@gmail.com> on 2018/04/12 02:02:11 UTC

why acquire a lock in TSimpleServer implementation for Go?

hi, all:
    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.