THRIFT-4537: TSimpleServer can exit AcceptLoop() without releasing lock
Client: go
This closes #1523.
diff --git a/lib/go/thrift/simple_server.go b/lib/go/thrift/simple_server.go
index 37081bd..6035802 100644
--- a/lib/go/thrift/simple_server.go
+++ b/lib/go/thrift/simple_server.go
@@ -125,26 +125,38 @@
return p.serverTransport.Listen()
}
+func (p *TSimpleServer) innerAccept() (int32, error) {
+ client, err := p.serverTransport.Accept()
+ p.mu.Lock()
+ defer p.mu.Unlock()
+ closed := atomic.LoadInt32(&p.closed)
+ if closed != 0 {
+ return closed, nil
+ }
+ if err != nil {
+ return 0, 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)
+ }
+ }()
+ }
+ return 0, nil
+}
+
func (p *TSimpleServer) AcceptLoop() error {
for {
- client, err := p.serverTransport.Accept()
- p.mu.Lock()
- if atomic.LoadInt32(&p.closed) != 0 {
- return nil
- }
+ closed, err := p.innerAccept()
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)
- }
- }()
+ if closed != 0 {
+ return nil
}
- p.mu.Unlock()
}
}
diff --git a/lib/go/thrift/simple_server_test.go b/lib/go/thrift/simple_server_test.go
index 25702e4..58149a8 100644
--- a/lib/go/thrift/simple_server_test.go
+++ b/lib/go/thrift/simple_server_test.go
@@ -21,7 +21,8 @@
import (
"testing"
- "time"
+ "errors"
+ "runtime"
)
type mockServerTransport struct {
@@ -122,6 +123,34 @@
serv := NewTSimpleServer2(proc, trans)
go serv.Serve()
- time.Sleep(1)
+ runtime.Gosched()
+ serv.Stop()
+}
+
+func TestNoHangDuringStopFromDanglingLockAcquireDuringAcceptLoop(t *testing.T) {
+ proc := &mockProcessor{
+ ProcessFunc: func(in, out TProtocol) (bool, TException) {
+ return false, nil
+ },
+ }
+
+ trans := &mockServerTransport{
+ ListenFunc: func() error {
+ return nil
+ },
+ AcceptFunc: func() (TTransport, error) {
+ return nil, errors.New("no sir")
+ },
+ CloseFunc: func() error {
+ return nil
+ },
+ InterruptFunc: func() error {
+ return nil
+ },
+ }
+
+ serv := NewTSimpleServer2(proc, trans)
+ go serv.Serve()
+ runtime.Gosched()
serv.Stop()
}