[ https://issues.apache.org/jira/browse/THRIFT-2157?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15131098#comment-15131098 ]
John Sirois commented on THRIFT-2157: ------------------------------------- [~jensg] or [~mbreslow] Is there more coming in this fix? As things stand the ClassCastException will still be thrown fwict so the issue should be re-opened. My reasoning: >From a fresh master: {noformat} $ git log -1 commit d094e79de7e0bd61320f006c83c0de669363bce8 Author: Nobuaki Sukegawa <ns...@apache.org> Date: Mon Feb 1 21:47:49 2016 +0900 THRIFT-3592 Add basic test client This closes #830 {noformat} The cast to TBase is still present in the compiler generated code; yet TApplicationException is only a TSerializable now after this fix, not a TBase: {noformat} $ git grep -A1 "(org.apache.thrift.TBase)" compiler/cpp/src/generate/t_java_generator.cc compiler/cpp/src/generate/t_java_generator.cc: indent(f_service_) << "msg = (org.apache.thrift.TBase)new " compiler/cpp/src/generate/t_java_generator.cc- "org.apache.thrift.TApplicationException(org.apache.thrift." $ git grep "class TApplicationException" -- lib/java/ lib/java/src/org/apache/thrift/TApplicationException.java:public class TApplicationException extends TException implements TSerializable { $ git grep "class TException" -- lib/java/ lib/java/src/org/apache/thrift/TException.java:public class TException extends Exception { $ git grep "interface TSerializable" -- lib/java/ lib/java/src/org/apache/thrift/TSerializable.java:public interface TSerializable { {noformat} So [~mbreslow]'s change sets the stage for the full fix but does not complete it fwict. To complete this fix a change to {{compiler/cpp/src/generate/t_java_generator.cc}} code generation for {{AsyncProcessFunction.getResultHandler}}'s {{onError}} implementation is still needed as well as a fix of the signature of {{AsyncProcessFunction.sendResponse}}. A minimal finishing of the fix: {noformat} $ git diff --full-index diff --git a/compiler/cpp/src/generate/t_java_generator.cc b/compiler/cpp/src/generate/t_java_generator.cc index 7c610fb8f025e8609d01cf8bc55f498d20076848..d93fd8a28a5d9eaac46215cc51a69f5a5ec17cd2 100644 --- a/compiler/cpp/src/generate/t_java_generator.cc +++ b/compiler/cpp/src/generate/t_java_generator.cc @@ -3301,7 +3301,7 @@ void t_java_generator::generate_process_async_function(t_service* tservice, t_fu if (!tfunction->is_oneway()) { indent(f_service_) << "byte msgType = org.apache.thrift.protocol.TMessageType.REPLY;" << endl; - indent(f_service_) << "org.apache.thrift.TBase msg;" << endl; + indent(f_service_) << "org.apache.thrift.TSerializable msg;" << endl; indent(f_service_) << resultname << " result = new " << resultname << "();" << endl; t_struct* xs = tfunction->get_xceptions(); @@ -3327,7 +3327,7 @@ void t_java_generator::generate_process_async_function(t_service* tservice, t_fu indent(f_service_) << "{" << endl; indent_up(); indent(f_service_) << "msgType = org.apache.thrift.protocol.TMessageType.EXCEPTION;" << endl; - indent(f_service_) << "msg = (org.apache.thrift.TBase)new " + indent(f_service_) << "msg = new " "org.apache.thrift.TApplicationException(org.apache.thrift." "TApplicationException.INTERNAL_ERROR, e.getMessage());" << endl; indent_down(); diff --git a/lib/java/src/org/apache/thrift/AsyncProcessFunction.java b/lib/java/src/org/apache/thrift/AsyncProcessFunction.java index 799e02d58ace7a7fc29e70168f4d785a0b61599f..8537e32518844b682a555e0dcbfd417086c31597 100644 --- a/lib/java/src/org/apache/thrift/AsyncProcessFunction.java +++ b/lib/java/src/org/apache/thrift/AsyncProcessFunction.java @@ -43,7 +43,7 @@ public abstract class AsyncProcessFunction<I, T, R> { return methodName; } - public void sendResponse(final AbstractNonblockingServer.AsyncFrameBuffer fb, final TBase result, final byte type, final int seqid) throws TException { + public void sendResponse(final AbstractNonblockingServer.AsyncFrameBuffer fb, final TSerializable result, final byte type, final int seqid) throws TException { TProtocol oprot = fb.getOutputProtocol(); oprot.writeMessageBegin(new TMessage(getMethodName(), type, seqid)); {noformat} That passes {{make check}} (well, go tests hang, but that also occurs on clean master :/, see [\[1\]|#1] below), but that said, it seems to me a unit test should be required to close out this issue as truly resolved. I'm happy to work up this missing patch with a unit test if everyone concurs the fix is incomplete. {anchor:1} \[1\] master go hang {noformat} make check ... /home/jsirois/.rvm/gems/ruby-2.1.4@global/bin/bundle exec /home/jsirois/.rvm/rubies/ruby-2.1.4/bin/ruby -I. test_suite.rb Loaded suite test_suite Started ............ Finished in 0.0015732 seconds. ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 12 tests, 25 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 7627.77 tests/s, 15891.18 assertions/s make[2]: Leaving directory '/home/jsirois/dev/3rdparty/thrift/test/rb' Making check in go make[2]: Entering directory '/home/jsirois/dev/3rdparty/thrift/test/go' Makefile:649: warning: overriding recipe for target 'check' Makefile:498: warning: ignoring old recipe for target 'check' mkdir -p src/gen ../../compiler/cpp/thrift -out src/gen --gen go:thrift_import=thrift ThriftTest.thrift [WARNING:/home/jsirois/dev/3rdparty/thrift/test/go/ThriftTest.thrift:83] The "byte" type is a compatibility alias for "i8". Use "i8" to emphasize the signedness of this type. [WARNING:/home/jsirois/dev/3rdparty/thrift/test/go/ThriftTest.thrift:44] No generator named 'noexist' could be found! [WARNING:/home/jsirois/dev/3rdparty/thrift/test/go/ThriftTest.thrift:46] cpp generator does not accept 'noexist' as sub-namespace! ../../compiler/cpp/thrift -out src/gen --gen go:thrift_import=thrift ../StressTest.thrift [WARNING:/home/jsirois/dev/3rdparty/thrift/test/StressTest.thrift:27] The "byte" type is a compatibility alias for "i8". Use "i8" to emphasize the signedness of this type. [WARNING:/home/jsirois/dev/3rdparty/thrift/test/StressTest.thrift:31] Consider using the more efficient "binary" type instead of "list<byte>". [WARNING:/home/jsirois/dev/3rdparty/thrift/test/StressTest.thrift:31] Consider using the more efficient "binary" type instead of "list<byte>". ln -nfs ../../../lib/go/thrift src/thrift GOPATH=`pwd` /usr/bin/go get github.com/golang/mock/gomock touch gopath GOPATH=`pwd` /usr/bin/go test -v common/... === RUN TestAllConnection [hangs here] {noformat} > generated code would cause ClassCastException > --------------------------------------------- > > Key: THRIFT-2157 > URL: https://issues.apache.org/jira/browse/THRIFT-2157 > Project: Thrift > Issue Type: Bug > Components: Java - Compiler > Affects Versions: 0.9.1 > Reporter: Dave Brosius > Assignee: Marc Breslow > Priority: Trivial > Fix For: 1.0 > > > Looking at the thrift generated code for cassandra, i'm seeing > msg = (org.apache.thrift.TBase)new > org.apache.thrift.TApplicationException(org.apache.thrift.TApplicationException.INTERNAL_ERROR, > e.getMessage()); > as seen here > https://git-wip-us.apache.org/repos/asf?p=cassandra.git;a=blob;f=interface/thrift/gen-java/org/apache/cassandra/thrift/Cassandra.java;h=837acfc0e964249fd62720420e8f1f85d966f1a3;hb=8f202895ab9e17c3d6bd4965924fd5f1ffc27f94#l6095 > i don't see how TApplicationException can be cast to TBase, and so i'd expect > a ClassCastException there. -- This message was sent by Atlassian JIRA (v6.3.4#6332)