[ https://issues.apache.org/jira/browse/THRIFT-4447?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16319207#comment-16319207 ]
ASF GitHub Bot commented on THRIFT-4447: ---------------------------------------- Github user johnboiles commented on the issue: https://github.com/apache/thrift/pull/1461 You're correct that it's a service extending another service; tbh I missed that too (I'm not the author of the `.thrift` files. I'm also generally new to thrift so still sorting out what should be generated). So basically the logic would be to only add `c: thrift.TClient` to the `*Client` struct if `extends.empty()` in the generator? I'll take a look and see how hard that looks. > 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)