Details
-
Bug
-
Status: Closed
-
Major
-
Resolution: Fixed
-
0.11.0
-
None
-
Patch Available
Description
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
type MyServiceClient struct {
c thrift.TClient
*MyServiceBaseClient
}
type MyServiceBaseClient struct {
c thrift.TClient
}
And also a method like this:
func NewMyServiceClientFactory(t thrift.TTransport, f thrift.TProtocolFactory) *MyServiceClient {
return &MyServiceClient{MyServiceBaseClient: NewMyServiceBaseClientFactory(t, f)}
}
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).
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
...
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.
src/tutorial/tutorial.go:477:33: bc.c undefined (cannot refer to unexported field or method c)
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.
Attachments
Issue Links
- links to
