You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "John Sirois (JIRA)" <ji...@apache.org> on 2016/02/03 21:47:40 UTC
[jira] [Commented] (THRIFT-2157) generated code would cause
ClassCastException
[ 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)