[
https://issues.apache.org/jira/browse/THRIFT-4447?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16321089#comment-16321089
]
ASF GitHub Bot commented on THRIFT-4447:
----------------------------------------
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1461
Travis was green so i've squashed it!
FYI for the future: GitHub can now automatically squash commits in PRs at
merge time. If that feature is enabled for the repo it shouldn't be necessary
for committers to squash their branch prior to merging, since everything can be
squashed via the GitHub UI:
> 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)
