buptubuntu commented on a change in pull request #2497:
URL: https://github.com/apache/thrift/pull/2497#discussion_r780653901
##########
File path: lib/go/thrift/simple_server_test.go
##########
@@ -154,3 +158,50 @@ func
TestNoHangDuringStopFromDanglingLockAcquireDuringAcceptLoop(t *testing.T) {
runtime.Gosched()
serv.Stop()
}
+
+func TestNoHangDuringStopFromClientNoDataSendDuringAcceptLoop(t *testing.T) {
+ ln, err := net.Listen("tcp", "localhost:0")
+ if err != nil {
+ t.Errorf("error when listen")
+ }
+ proc := &mockProcessor{
+ ProcessFunc: func(in, out TProtocol) (bool, TException) {
+ in.ReadMessageBegin(context.Background())
+ return false, nil
+ },
+ }
+
+ trans := &mockServerTransport{
+ ListenFunc: func() error {
+ return nil
+ },
+ AcceptFunc: func() (TTransport, error) {
+ conn, err := ln.Accept()
+ if err != nil {
+ // t.Errorf("error accept connection")
+ return nil, err
+ }
+ return NewTSocketFromConnTimeout(conn, 0), nil
+ },
+ CloseFunc: func() error {
+ return nil
+ },
+ InterruptFunc: func() error {
+ return ln.Close()
+ },
+ }
+
+ serv := NewTSimpleServer2(proc, trans)
+ go serv.Serve()
+
+ port := strings.Split(ln.Addr().String(), ":")[1]
+ netConn, err := net.Dial("tcp", "localhost:"+port)
+ if err != nil || netConn == nil {
+ t.Errorf("error when dial server")
+ }
+ time.Sleep(time.Second)
Review comment:
done
##########
File path: lib/go/thrift/socket.go
##########
@@ -195,8 +195,9 @@ func (p *TSocket) IsOpen() bool {
// Closes the socket.
func (p *TSocket) Close() error {
// Close the socket
- if p.conn != nil {
- err := p.conn.Close()
+ conn := p.conn
Review comment:
OK,i have undo the change
##########
File path: lib/go/thrift/simple_server.go
##########
@@ -234,7 +250,9 @@ func (p *TSimpleServer) Stop() error {
}
atomic.StoreInt32(&p.closed, 1)
p.serverTransport.Interrupt()
+ close(p.stopChan)
p.wg.Wait()
+ p.stopChan = make(chan struct{})
Review comment:
I think it is better to init the stopChan in construction, if not
TestMultipleStop will panic because of nil chan
What's more, current HEAD allows multi **Serve**, if we make it in
**Serve**，the stopChan may change and will never be closed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
