[ 
https://issues.apache.org/jira/browse/THRIFT-4240?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16065319#comment-16065319
 ] 

ASF GitHub Bot commented on THRIFT-4240:
----------------------------------------

GitHub user zwass opened a pull request:

    https://github.com/apache/thrift/pull/1301

    THRIFT-4240 Fix use of sync.Once in Go TSimpleServer Stop() method

    A package global sync.Once was used, which ensured that Stop() could only be
    called once *per process*. The correct semantics should be once *per 
server*.
    
    This commit makes the sync.Once a member of the server struct so that the
    correct semantics are applied.
    
    https://issues.apache.org/jira/browse/THRIFT-4240

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/zwass/thrift THRIFT-4240

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/thrift/pull/1301.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1301
    
----
commit 3f0444f8292a6ca1889e9812b805759eb8a74255
Author: Zachary Wasserman <zachwass2...@gmail.com>
Date:   2017-06-27T19:03:39Z

    THRIFT-4240 Fix use of sync.Once in Go TSimpleServer Stop() method
    
    A package global sync.Once was used, which ensured that Stop() could only be
    called once *per process*. The correct semantics should be once *per 
server*.
    
    This commit makes the sync.Once a member of the server struct so that the
    correct semantics are applied.
    
    https://issues.apache.org/jira/browse/THRIFT-4240

----


> Go TSimpleServer does not close properly
> ----------------------------------------
>
>                 Key: THRIFT-4240
>                 URL: https://issues.apache.org/jira/browse/THRIFT-4240
>             Project: Thrift
>          Issue Type: Bug
>          Components: Go - Library
>    Affects Versions: 0.10.0
>            Reporter: Zach Wasserman
>
> Improper use of {{sync.Once}} results in only the first call to {{Close()}} 
> in a given process actually executing.
> The {{sync.Once}} object should be a struct member, and not package global.
> Test:
> {code}
> type mockProcessor struct {
>       ProcessFunc func(in, out TProtocol) (bool, TException)
> }
> func (m *mockProcessor) Process(in, out TProtocol) (bool, TException) {
>       return m.ProcessFunc(in, out)
> }
> type mockServerTransport struct {
>       ListenFunc    func() error
>       AcceptFunc    func() (TTransport, error)
>       CloseFunc     func() error
>       InterruptFunc func() error
> }
> func (m *mockServerTransport) Listen() error {
>       return m.ListenFunc()
> }
> func (m *mockServerTransport) Accept() (TTransport, error) {
>       return m.AcceptFunc()
> }
> func (m *mockServerTransport) Close() error {
>       return m.CloseFunc()
> }
> func (m *mockServerTransport) Interrupt() error {
>       return m.InterruptFunc()
> }
> func TestMultipleStop(t *testing.T) {
>       proc := &mockProcessor{
>               ProcessFunc: func(in, out TProtocol) (bool, TException) {
>                       return false, nil
>               },
>       }
>       var interruptCalled bool
>       c := make(chan struct{})
>       trans := &mockServerTransport{
>               ListenFunc: func() error {
>                       return nil
>               },
>               AcceptFunc: func() (TTransport, error) {
>                       <-c
>                       return nil, nil
>               },
>               CloseFunc: func() error {
>                       c <- struct{}{}
>                       return nil
>               },
>               InterruptFunc: func() error {
>                       interruptCalled = true
>                       return nil
>               },
>       }
>       serv := NewTSimpleServer2(proc, trans)
>       go serv.Serve()
>       serv.Stop()
>       if !interruptCalled {
>               t.Error("first server transport should have been interrupted")
>       }
>       serv = NewTSimpleServer2(proc, trans)
>       interruptCalled = false
>       go serv.Serve()
>       serv.Stop()
>       if !interruptCalled {
>               t.Error("second server transport should have been interrupted")
>       }
> }
> {code}
> Output:
> {code}
> go test -race -run TestMultipleStop
> --- FAIL: TestMultipleStop (0.00s)
>       simple_server_test.go:96: second server transport should have been 
> interrupted
> FAIL
> exit status 1
> FAIL  
> github.com/kolide/launcher/vendor/git.apache.org/thrift.git/lib/go/thrift     
>   0.020s
> {code}



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to