You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by dc...@apache.org on 2018/03/30 09:04:03 UTC
thrift git commit: THRIFT-4537: TSimpleServer can exit AcceptLoop()
without releasing lock
Repository: thrift
Updated Branches:
refs/heads/master 930428438 -> 8a83b041d
THRIFT-4537: TSimpleServer can exit AcceptLoop() without releasing lock
Client: go
This closes #1523.
Project: http://git-wip-us.apache.org/repos/asf/thrift/repo
Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/8a83b041
Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/8a83b041
Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/8a83b041
Branch: refs/heads/master
Commit: 8a83b041d20671c3fae9528d5ac1f5413cad7c5a
Parents: 9304284
Author: Matthew Pound <mw...@signalfx.com>
Authored: Thu Mar 29 14:03:50 2018 -0700
Committer: D. Can Celasun <ca...@dcc.im>
Committed: Fri Mar 30 08:48:15 2018 +0200
----------------------------------------------------------------------
lib/go/thrift/simple_server.go | 40 +++++++++++++++++++++-----------
lib/go/thrift/simple_server_test.go | 33 ++++++++++++++++++++++++--
2 files changed, 57 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/thrift/blob/8a83b041/lib/go/thrift/simple_server.go
----------------------------------------------------------------------
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 @@ func (p *TSimpleServer) Listen() error {
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()
}
}
http://git-wip-us.apache.org/repos/asf/thrift/blob/8a83b041/lib/go/thrift/simple_server_test.go
----------------------------------------------------------------------
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 @@ package thrift
import (
"testing"
- "time"
+ "errors"
+ "runtime"
)
type mockServerTransport struct {
@@ -122,6 +123,34 @@ func TestWaitRace(t *testing.T) {
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()
}