John Boiles created THRIFT-4447:
-----------------------------------

             Summary: Golang: Panic on p.c.Call when using deprecated 
initializers
                 Key: THRIFT-4447
                 URL: https://issues.apache.org/jira/browse/THRIFT-4447
             Project: Thrift
          Issue Type: Bug
          Components: Go - Compiler
    Affects Versions: 0.11.0
            Reporter: John Boiles


Latest thrift:master can cause panics when using deprecated `New*ClientFactory` 
and `New*ClientProtocol` functions. This happens because both the Client and 
the BaseClient have an instance of a {{thrift.TClient}}. The deprecated methods 
initialize the BaseClient's TClient, but other methods use the Client's 
{{TClient}}.

For example, current thrift master generates structs like this

{code}
type MyServiceClient struct {
        c thrift.TClient
        *MyServiceBaseClient
}

type MyServiceBaseClient struct {
        c thrift.TClient
}
{code}

And also a method like this:

{code}
func NewMyServiceClientFactory(t thrift.TTransport, f thrift.TProtocolFactory) 
*MyServiceClient {
      return &MyServiceClient{MyServiceBaseClient: 
NewMyServiceBaseClientFactory(t, f)}
}
{code}

If that method is used, later calls to service methods will panic, since 
{{p.c}} is nil (the actual client was stored in {{p.BaseMyServiceClient.c}}).

{code}
func (p *MyServiceClient) DoStuff(ctx context.Context, request *DoStuffRequest) 
(r *DoStuffResponse, err error) {
        var _args139 DoStuffArgs
        _args139.Request = request
        var _result140 DoStuffResult
        if err = p.c.Call(ctx, "do_stuff", &_args139, &_result140); err != nil 
{ // PANIC
        ...
{code}

In progress fix here :https://github.com/apache/thrift/pull/1461. The fix in 
this PR merely sets both instances of {{TClient}} (which is what happens in the 
non-deprecated {{New*Client}} function).

This patch currently fails {{make -k check}} however, since 
{{src/tutorial/tutorial.go}} tries to access a different package's version of 
the BaseClient.

{code}
src/tutorial/tutorial.go:477:33: bc.c undefined (cannot refer to unexported 
field or method c)
{code}

The fix for that test could possibly be to expose the BaseClient's instance of 
{{c}} (by making it a capital and thus exported {{C}}), or adding an accessor 
method {{C()}} or {{Client()}}.

Possibly a better fix would be to either remove these deprecated methods, or 
figure out which {{TClient}} is the correct one to set.

I'm not sure which is preferred, so I'm hoping to get some feedback/input here.



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

Reply via email to