Author: bryanduxbury
Date: Wed Apr 1 20:10:15 2009
New Revision: 761037
URL: http://svn.apache.org/viewvc?rev=761037&view=rev
Log:
THRIFT-417. rb: BufferedTransport can enter an infinite loop
Switch native proto implementations to use read_all instead of read. Add a
bunch of tests to verify.
Also:
- removed some commented code in binary_protocol_accelerated.c
- struct.c was missing one of the possible native method calls
- updates gem manifest (included files that didn't exist)
- fixed svn:ignores of test/rb/gen-rb and lib/java/gen-java
Modified:
incubator/thrift/trunk/lib/java/ (props changed)
incubator/thrift/trunk/lib/java/test/org/apache/thrift/test/TCompactProtocolTest.java
incubator/thrift/trunk/lib/rb/Manifest
incubator/thrift/trunk/lib/rb/ext/binary_protocol_accelerated.c
incubator/thrift/trunk/lib/rb/ext/constants.h
incubator/thrift/trunk/lib/rb/ext/macros.h
incubator/thrift/trunk/lib/rb/ext/struct.c
incubator/thrift/trunk/lib/rb/ext/thrift_native.c
incubator/thrift/trunk/lib/rb/spec/binaryprotocol_spec_shared.rb
incubator/thrift/trunk/lib/rb/spec/spec_helper.rb
incubator/thrift/trunk/test/DebugProtoTest.thrift
incubator/thrift/trunk/test/rb/ (props changed)
Propchange: incubator/thrift/trunk/lib/java/
------------------------------------------------------------------------------
--- svn:ignore (original)
+++ svn:ignore Wed Apr 1 20:10:15 2009
@@ -1,4 +1,4 @@
-gen-java
+gen-java
gen-javabean
build
*.jar
Modified:
incubator/thrift/trunk/lib/java/test/org/apache/thrift/test/TCompactProtocolTest.java
URL:
http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/java/test/org/apache/thrift/test/TCompactProtocolTest.java?rev=761037&r1=761036&r2=761037&view=diff
==============================================================================
---
incubator/thrift/trunk/lib/java/test/org/apache/thrift/test/TCompactProtocolTest.java
(original)
+++
incubator/thrift/trunk/lib/java/test/org/apache/thrift/test/TCompactProtocolTest.java
Wed Apr 1 20:10:15 2009
@@ -365,6 +365,21 @@
public int Janky(int i32arg) throws TException {
return i32arg * 2;
}
+
+ public int primitiveMethod() throws TException {
+ // TODO Auto-generated method stub
+ return 0;
+ }
+
+ public CompactProtoTestStruct structMethod() throws TException {
+ // TODO Auto-generated method stub
+ return null;
+ }
+
+ public void voidMethod() throws TException {
+ // TODO Auto-generated method stub
+
+ }
};
Srv.Processor testProcessor = new Srv.Processor(handler);
Modified: incubator/thrift/trunk/lib/rb/Manifest
URL:
http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/rb/Manifest?rev=761037&r1=761036&r2=761037&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/rb/Manifest (original)
+++ incubator/thrift/trunk/lib/rb/Manifest Wed Apr 1 20:10:15 2009
@@ -7,7 +7,6 @@
benchmark/server.rb
benchmark/thin_server.rb
CHANGELOG
-COPYING
ext/binary_protocol_accelerated.c
ext/binary_protocol_accelerated.h
ext/compact_protocol.c
@@ -43,7 +42,6 @@
lib/thrift/transport.rb
lib/thrift/types.rb
lib/thrift.rb
-LICENSE
Manifest
README
spec/binaryprotocol_spec.rb
Modified: incubator/thrift/trunk/lib/rb/ext/binary_protocol_accelerated.c
URL:
http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/rb/ext/binary_protocol_accelerated.c?rev=761037&r1=761036&r2=761037&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/rb/ext/binary_protocol_accelerated.c (original)
+++ incubator/thrift/trunk/lib/rb/ext/binary_protocol_accelerated.c Wed Apr 1
20:10:15 2009
@@ -336,7 +336,7 @@
VALUE rb_thrift_binary_proto_read_bool(VALUE self) {
char byte = read_byte_direct(self);
- return byte == 1 ? Qtrue : Qfalse;
+ return byte != 0 ? Qtrue : Qfalse;
}
VALUE rb_thrift_binary_proto_read_byte(VALUE self) {
Modified: incubator/thrift/trunk/lib/rb/ext/constants.h
URL:
http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/rb/ext/constants.h?rev=761037&r1=761036&r2=761037&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/rb/ext/constants.h (original)
+++ incubator/thrift/trunk/lib/rb/ext/constants.h Wed Apr 1 20:10:15 2009
@@ -73,7 +73,7 @@
extern ID write_field_stop_method_id;
extern ID skip_method_id;
extern ID write_method_id;
-extern ID read_method_id;
+extern ID read_all_method_id;
extern ID native_qmark_method_id;
extern ID fields_const_id;
Modified: incubator/thrift/trunk/lib/rb/ext/macros.h
URL:
http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/rb/ext/macros.h?rev=761037&r1=761036&r2=761037&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/rb/ext/macros.h (original)
+++ incubator/thrift/trunk/lib/rb/ext/macros.h Wed Apr 1 20:10:15 2009
@@ -22,7 +22,7 @@
#define GET_STRICT_WRITE(obj) rb_ivar_get(obj, strict_write_ivar_id)
#define WRITE(obj, data, length) rb_funcall(obj, write_method_id, 1,
rb_str_new(data, length))
#define CHECK_NIL(obj) if (NIL_P(obj)) { rb_raise(rb_eStandardError, "nil
argument not allowed!");}
-#define READ(obj, length) rb_funcall(GET_TRANSPORT(obj), read_method_id, 1,
INT2FIX(length))
+#define READ(obj, length) rb_funcall(GET_TRANSPORT(obj), read_all_method_id,
1, INT2FIX(length))
#ifndef RFLOAT_VALUE
# define RFLOAT_VALUE(v) RFLOAT(rb_Float(v))->value
Modified: incubator/thrift/trunk/lib/rb/ext/struct.c
URL:
http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/rb/ext/struct.c?rev=761037&r1=761036&r2=761037&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/rb/ext/struct.c (original)
+++ incubator/thrift/trunk/lib/rb/ext/struct.c Wed Apr 1 20:10:15 2009
@@ -416,11 +416,6 @@
// call validate
rb_funcall(self, validate_method_id, 0);
- // if (rb_funcall(protocol, native_qmark_method_id, 0) == Qtrue) {
- // set_native_proto_function_pointers(protocol);
- // } else {
- // set_default_proto_function_pointers();
- // }
check_native_proto_method_table(protocol);
// write struct begin
@@ -565,7 +560,7 @@
// read each field
while (true) {
- VALUE field_header = rb_funcall(protocol, read_field_begin_method_id, 0);
+ VALUE field_header = mt->read_field_begin(protocol);
VALUE field_type_value = rb_ary_entry(field_header, 1);
int field_type = FIX2INT(field_type_value);
Modified: incubator/thrift/trunk/lib/rb/ext/thrift_native.c
URL:
http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/rb/ext/thrift_native.c?rev=761037&r1=761036&r2=761037&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/rb/ext/thrift_native.c (original)
+++ incubator/thrift/trunk/lib/rb/ext/thrift_native.c Wed Apr 1 20:10:15 2009
@@ -87,7 +87,7 @@
ID write_field_stop_method_id;
ID skip_method_id;
ID write_method_id;
-ID read_method_id;
+ID read_all_method_id;
ID native_qmark_method_id;
// constant ids
@@ -169,7 +169,7 @@
write_field_stop_method_id = rb_intern("write_field_stop");
skip_method_id = rb_intern("skip");
write_method_id = rb_intern("write");
- read_method_id = rb_intern("read");
+ read_all_method_id = rb_intern("read_all");
native_qmark_method_id = rb_intern("native?");
// constant ids
Modified: incubator/thrift/trunk/lib/rb/spec/binaryprotocol_spec_shared.rb
URL:
http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/rb/spec/binaryprotocol_spec_shared.rb?rev=761037&r1=761036&r2=761037&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/rb/spec/binaryprotocol_spec_shared.rb (original)
+++ incubator/thrift/trunk/lib/rb/spec/binaryprotocol_spec_shared.rb Wed Apr 1
20:10:15 2009
@@ -299,4 +299,77 @@
@trans.write([str.size].pack("N") + str)
@prot.read_string.should == str
end
+
+ it "should perform a complete rpc with no args or return" do
+ srv_test(
+ proc {|client| client.send_voidMethod()},
+ proc {|client| client.recv_voidMethod.should == nil}
+ )
+ end
+
+ it "should perform a complete rpc with a primitive return type" do
+ srv_test(
+ proc {|client| client.send_primitiveMethod()},
+ proc {|client| client.recv_primitiveMethod.should == 1}
+ )
+ end
+
+ it "should perform a complete rpc with a struct return type" do
+ srv_test(
+ proc {|client| client.send_structMethod()},
+ proc {|client|
+ result = client.recv_structMethod
+ result.set_byte_map = nil
+ result.map_byte_map = nil
+ result.should == Fixtures::COMPACT_PROTOCOL_TEST_STRUCT
+ }
+ )
+ end
+
+ def get_socket_connection
+ server = Thrift::ServerSocket.new("localhost", 9090)
+ server.listen
+
+ clientside = Thrift::Socket.new("localhost", 9090)
+ clientside.open
+ serverside = server.accept
+ [clientside, serverside, server]
+ end
+
+ def srv_test(firstblock, secondblock)
+ clientside, serverside, server = get_socket_connection
+
+ clientproto = protocol_class.new(clientside)
+ serverproto = protocol_class.new(serverside)
+
+ processor = Srv::Processor.new(SrvHandler.new)
+
+ client = Srv::Client.new(clientproto, clientproto)
+
+ # first block
+ firstblock.call(client)
+
+ processor.process(serverproto, serverproto)
+
+ # second block
+ secondblock.call(client)
+ ensure
+ clientside.close
+ serverside.close
+ server.close
+ end
+
+ class SrvHandler
+ def voidMethod()
+ end
+
+ def primitiveMethod
+ 1
+ end
+
+ def structMethod
+ Fixtures::COMPACT_PROTOCOL_TEST_STRUCT
+ end
+ end
+
end
Modified: incubator/thrift/trunk/lib/rb/spec/spec_helper.rb
URL:
http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/rb/spec/spec_helper.rb?rev=761037&r1=761036&r2=761037&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/rb/spec/spec_helper.rb (original)
+++ incubator/thrift/trunk/lib/rb/spec/spec_helper.rb Wed Apr 1 20:10:15 2009
@@ -48,4 +48,6 @@
module Fixtures
COMPACT_PROTOCOL_TEST_STRUCT = CompactProtoTestStruct.new(:a_binary =>
[0,1,2,3,4,5,6,7,8].pack('c*'))
+ COMPACT_PROTOCOL_TEST_STRUCT.set_byte_map = nil
+ COMPACT_PROTOCOL_TEST_STRUCT.map_byte_map = nil
end
\ No newline at end of file
Modified: incubator/thrift/trunk/test/DebugProtoTest.thrift
URL:
http://svn.apache.org/viewvc/incubator/thrift/trunk/test/DebugProtoTest.thrift?rev=761037&r1=761036&r2=761037&view=diff
==============================================================================
--- incubator/thrift/trunk/test/DebugProtoTest.thrift (original)
+++ incubator/thrift/trunk/test/DebugProtoTest.thrift Wed Apr 1 20:10:15 2009
@@ -97,25 +97,6 @@
7: binary b6,
}
-service Srv {
- i32 Janky(i32 arg)
-}
-
-service Inherited extends Srv {
- i32 identity(i32 arg)
-}
-
-service EmptyService {}
-
-// The only purpose of this thing is to increase the size of the generated code
-// so that ZlibTest has more highly compressible data to play with.
-struct BlowUp {
- 1: map<list<i32>,set<map<i32,string>>> b1;
- 2: map<list<i32>,set<map<i32,string>>> b2;
- 3: map<list<i32>,set<map<i32,string>>> b3;
- 4: map<list<i32>,set<map<i32,string>>> b4;
-}
-
struct CompactProtoTestStruct {
// primitive fields
1: byte a_byte = 127;
@@ -178,3 +159,31 @@
48: map<byte, set<byte>> byte_set_map = {0 : [], 1 : [1], 2 : [1, 2]};
49: map<byte, list<byte>> byte_list_map = {0 : [], 1 : [1], 2 : [1,
2]};
}
+
+
+
+service Srv {
+ i32 Janky(i32 arg);
+
+ // return type only methods
+
+ void voidMethod();
+ i32 primitiveMethod();
+ CompactProtoTestStruct structMethod();
+}
+
+service Inherited extends Srv {
+ i32 identity(i32 arg)
+}
+
+service EmptyService {}
+
+// The only purpose of this thing is to increase the size of the generated code
+// so that ZlibTest has more highly compressible data to play with.
+struct BlowUp {
+ 1: map<list<i32>,set<map<i32,string>>> b1;
+ 2: map<list<i32>,set<map<i32,string>>> b2;
+ 3: map<list<i32>,set<map<i32,string>>> b3;
+ 4: map<list<i32>,set<map<i32,string>>> b4;
+}
+
Propchange: incubator/thrift/trunk/test/rb/
------------------------------------------------------------------------------
--- svn:ignore (original)
+++ svn:ignore Wed Apr 1 20:10:15 2009
@@ -1,2 +1,2 @@
-
+gen-rb
Makefile*
