[jira] [Updated] (THRIFT-4366) upgrade thrift lib to 0.10 ,coredump at readMessageBegin

2017-10-26 Thread xiaomingzhongguo (JIRA)

 [ 
https://issues.apache.org/jira/browse/THRIFT-4366?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

xiaomingzhongguo updated THRIFT-4366:
-
Summary: upgrade thrift lib to 0.10 ,coredump at readMessageBegin   (was: 
upgrade to 0.10 CPP ,when readMessageBegin coredump )

> upgrade thrift lib to 0.10 ,coredump at readMessageBegin 
> -
>
> Key: THRIFT-4366
> URL: https://issues.apache.org/jira/browse/THRIFT-4366
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Compiler
>Affects Versions: 0.10.0
> Environment: thrift 0.10 CPP 
>Reporter: xiaomingzhongguo
>Priority: Critical
>  Labels: performance
>
> (gdb) bt
> #0  0x7f34d99c85ae in memcpy () from /lib64/libc.so.6
> #1  0x0100f9fb in std::basic_string std::allocator >::_M_replace_safe(unsigned long, unsigned long, char 
> const*, unsigned long) ()
> at 
> /data/davy/26lib_source/gcc-4.8.2/build/x86_64-linux-gnu/libstdc++-v3/include/bits/char_traits.h:271
> #2  0x009cec48 in unsigned int 
> apache::thrift::protocol::TBinaryProtocolT  
> apache::thrift::protocol::TNetworkBigEndian>::readStringBody  std::char_traits, std::allocator > >(std::basic_string std::char_traits, std::allocator >&, int) () at 
> /usr/local/include/thrift/protocol/TBinaryProtocol.tcc:441
> #3  0x009ce34f in 
> apache::thrift::protocol::TBinaryProtocolT  
> apache::thrift::protocol::TNetworkBigEndian>::readMessageBegin(std::basic_string  std::char_traits, std::allocator >&, 
> apache::thrift::protocol::TMessageType&, int&) () at 
> /usr/local/include/thrift/protocol/TBinaryProtocol.tcc:223
> #4  0x009cd798 in 
> apache::thrift::protocol::TVirtualProtocol  apache::thrift::protocol::TNetworkBigEndian>, 
> apache::thrift::protocol::TProtocolDefaults>::readMessageBegin_virt(std::basic_string  std::char_traits, std::allocator >&, 
> apache::thrift::protocol::TMessageType&, int&) () at 
> /usr/local/include/thrift/protocol/TVirtualProtocol.h:403
> #5  0x009d047c in 
> apache::thrift::protocol::TProtocol::readMessageBegin(std::basic_string std::char_traits, std::allocator >&, 
> apache::thrift::protocol::TMessageType&, int&) ()
> at /usr/local/include/thrift/protocol/TProtocol.h:436
> #6  0x009d09b9 in 
> apache::thrift::TDispatchProcessor::process(boost::shared_ptr,
>  boost::shared_ptr, void*) ()
> at /usr/local/include/thrift/TDispatchProcessor.h:114
> #7  0x00ddd96f in apache::thrift::server::TConnectedClient::run() () 
> at src/thrift/server/TConnectedClient.cpp:62
> #8  0x00dd6ef8 in 
> apache::thrift::server::TThreadedServer::TConnectedClientRunner::run() () at 
> src/thrift/server/TThreadedServer.cpp:147
> #9  0x00dd9891 in 
> apache::thrift::concurrency::PthreadThread::threadMain(void*) () at 
> src/thrift/concurrency/PosixThreadFactory.cpp:208
> #10 0x7f34da66d7f1 in start_thread () from /lib64/libpthread.so.0
> #11 0x7f34d9a25ccd in clone () from /lib64/libc.so.6



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Updated] (THRIFT-4366) upgrade to 0.10 CPP ,when readMessageBegin coredump

2017-10-26 Thread xiaomingzhongguo (JIRA)

 [ 
https://issues.apache.org/jira/browse/THRIFT-4366?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

xiaomingzhongguo updated THRIFT-4366:
-
Priority: Critical  (was: Blocker)

> upgrade to 0.10 CPP ,when readMessageBegin coredump 
> 
>
> Key: THRIFT-4366
> URL: https://issues.apache.org/jira/browse/THRIFT-4366
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Compiler
>Affects Versions: 0.10.0
> Environment: thrift 0.10 CPP 
>Reporter: xiaomingzhongguo
>Priority: Critical
>  Labels: performance
>
> (gdb) bt
> #0  0x7f34d99c85ae in memcpy () from /lib64/libc.so.6
> #1  0x0100f9fb in std::basic_string std::allocator >::_M_replace_safe(unsigned long, unsigned long, char 
> const*, unsigned long) ()
> at 
> /data/davy/26lib_source/gcc-4.8.2/build/x86_64-linux-gnu/libstdc++-v3/include/bits/char_traits.h:271
> #2  0x009cec48 in unsigned int 
> apache::thrift::protocol::TBinaryProtocolT  
> apache::thrift::protocol::TNetworkBigEndian>::readStringBody  std::char_traits, std::allocator > >(std::basic_string std::char_traits, std::allocator >&, int) () at 
> /usr/local/include/thrift/protocol/TBinaryProtocol.tcc:441
> #3  0x009ce34f in 
> apache::thrift::protocol::TBinaryProtocolT  
> apache::thrift::protocol::TNetworkBigEndian>::readMessageBegin(std::basic_string  std::char_traits, std::allocator >&, 
> apache::thrift::protocol::TMessageType&, int&) () at 
> /usr/local/include/thrift/protocol/TBinaryProtocol.tcc:223
> #4  0x009cd798 in 
> apache::thrift::protocol::TVirtualProtocol  apache::thrift::protocol::TNetworkBigEndian>, 
> apache::thrift::protocol::TProtocolDefaults>::readMessageBegin_virt(std::basic_string  std::char_traits, std::allocator >&, 
> apache::thrift::protocol::TMessageType&, int&) () at 
> /usr/local/include/thrift/protocol/TVirtualProtocol.h:403
> #5  0x009d047c in 
> apache::thrift::protocol::TProtocol::readMessageBegin(std::basic_string std::char_traits, std::allocator >&, 
> apache::thrift::protocol::TMessageType&, int&) ()
> at /usr/local/include/thrift/protocol/TProtocol.h:436
> #6  0x009d09b9 in 
> apache::thrift::TDispatchProcessor::process(boost::shared_ptr,
>  boost::shared_ptr, void*) ()
> at /usr/local/include/thrift/TDispatchProcessor.h:114
> #7  0x00ddd96f in apache::thrift::server::TConnectedClient::run() () 
> at src/thrift/server/TConnectedClient.cpp:62
> #8  0x00dd6ef8 in 
> apache::thrift::server::TThreadedServer::TConnectedClientRunner::run() () at 
> src/thrift/server/TThreadedServer.cpp:147
> #9  0x00dd9891 in 
> apache::thrift::concurrency::PthreadThread::threadMain(void*) () at 
> src/thrift/concurrency/PosixThreadFactory.cpp:208
> #10 0x7f34da66d7f1 in start_thread () from /lib64/libpthread.so.0
> #11 0x7f34d9a25ccd in clone () from /lib64/libc.so.6



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-4373) Extending Thrift class results in "Attempt serialize from non-Thrift object"

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4373?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16221465#comment-16221465
 ] 

ASF GitHub Bot commented on THRIFT-4373:


Github user sokac commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1401#discussion_r147295660
  
--- Diff: lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp ---
@@ -699,6 +702,9 @@ void binary_serialize_hashtable_key(int8_t keytype, 
PHPOutputTransport& transpor
 
 static
 void binary_serialize(int8_t thrift_typeID, PHPOutputTransport& transport, 
zval* value, HashTable* fieldspec) {
+  if (value) {
--- End diff --

removed if statement


> Extending Thrift class results in "Attempt serialize from non-Thrift object"
> 
>
> Key: THRIFT-4373
> URL: https://issues.apache.org/jira/browse/THRIFT-4373
> Project: Thrift
>  Issue Type: Bug
>  Components: PHP - Library
>Affects Versions: 0.10.0
> Environment: Linux 4.13.3-1-ARCH
> PHP 7.1.10
>Reporter: Josip Sokcevic
> Attachments: 0001-THRIFT-4373-Derefer-PHP-zval-_TSPEC.patch
>
>
> This happens when using php extension. thrift_protocol_write_binary will 
> check Z_TYPE_P of spec and expects to be array (IS_ARRAY). However, PHP7 will 
> set it as reference (IS_REFERENCE).
> Example:
> {code}
> $s = new Serializer\TBinarySerializer();
> // Foo is a Thrift type class
> class FooExtended extends Foo {}
> $o = new Foo();
> $o2 = new FooExtended();
> // this works just fine:
> $s->serialize($o); // this uses thrift_protocol_write_binary if available
> // Next line throws \Thrift\Exception\TProtocolException if 
> thrift_protocol_write_binary is used
> // However, it doesn't break if no extension is available.
> $s->serialize($o);
> {code}
> Proposed solution is to dereference using ZVAL_DEREF before checking types 
> (attached). Alternative would be to mark struct type classes as final, but 
> that break compatibility.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-4373) Extending Thrift class results in "Attempt serialize from non-Thrift object"

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4373?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16221466#comment-16221466
 ] 

ASF GitHub Bot commented on THRIFT-4373:


Github user sokac commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1401#discussion_r147295690
  
--- Diff: lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp ---
@@ -709,6 +715,9 @@ void binary_serialize(int8_t thrift_typeID, 
PHPOutputTransport& transport, zval*
 throw_tprotocolexception("Attempt to send non-object type as a 
T_STRUCT", INVALID_DATA);
   }
   zval* spec = zend_read_static_property(Z_OBJCE_P(value), "_TSPEC", 
sizeof("_TSPEC")-1, true);
+  if (spec) {
--- End diff --

removed if statement


> Extending Thrift class results in "Attempt serialize from non-Thrift object"
> 
>
> Key: THRIFT-4373
> URL: https://issues.apache.org/jira/browse/THRIFT-4373
> Project: Thrift
>  Issue Type: Bug
>  Components: PHP - Library
>Affects Versions: 0.10.0
> Environment: Linux 4.13.3-1-ARCH
> PHP 7.1.10
>Reporter: Josip Sokcevic
> Attachments: 0001-THRIFT-4373-Derefer-PHP-zval-_TSPEC.patch
>
>
> This happens when using php extension. thrift_protocol_write_binary will 
> check Z_TYPE_P of spec and expects to be array (IS_ARRAY). However, PHP7 will 
> set it as reference (IS_REFERENCE).
> Example:
> {code}
> $s = new Serializer\TBinarySerializer();
> // Foo is a Thrift type class
> class FooExtended extends Foo {}
> $o = new Foo();
> $o2 = new FooExtended();
> // this works just fine:
> $s->serialize($o); // this uses thrift_protocol_write_binary if available
> // Next line throws \Thrift\Exception\TProtocolException if 
> thrift_protocol_write_binary is used
> // However, it doesn't break if no extension is available.
> $s->serialize($o);
> {code}
> Proposed solution is to dereference using ZVAL_DEREF before checking types 
> (attached). Alternative would be to mark struct type classes as final, but 
> that break compatibility.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1401: THRIFT-4373: Derefer PHP zval _TSPEC

2017-10-26 Thread sokac
Github user sokac commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1401#discussion_r147295690
  
--- Diff: lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp ---
@@ -709,6 +715,9 @@ void binary_serialize(int8_t thrift_typeID, 
PHPOutputTransport& transport, zval*
 throw_tprotocolexception("Attempt to send non-object type as a 
T_STRUCT", INVALID_DATA);
   }
   zval* spec = zend_read_static_property(Z_OBJCE_P(value), "_TSPEC", 
sizeof("_TSPEC")-1, true);
+  if (spec) {
--- End diff --

removed if statement


---


[GitHub] thrift pull request #1401: THRIFT-4373: Derefer PHP zval _TSPEC

2017-10-26 Thread sokac
Github user sokac commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1401#discussion_r147295660
  
--- Diff: lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp ---
@@ -699,6 +702,9 @@ void binary_serialize_hashtable_key(int8_t keytype, 
PHPOutputTransport& transpor
 
 static
 void binary_serialize(int8_t thrift_typeID, PHPOutputTransport& transport, 
zval* value, HashTable* fieldspec) {
+  if (value) {
--- End diff --

removed if statement


---


[jira] [Commented] (THRIFT-4373) Extending Thrift class results in "Attempt serialize from non-Thrift object"

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4373?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16221464#comment-16221464
 ] 

ASF GitHub Bot commented on THRIFT-4373:


Github user sokac commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1401#discussion_r147295629
  
--- Diff: lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp ---
@@ -537,6 +537,9 @@ void binary_deserialize(int8_t thrift_typeID, 
PHPInputTransport& transport, zval
   }
 
   zval* spec = zend_read_static_property(Z_OBJCE_P(return_value), 
"_TSPEC", sizeof("_TSPEC")-1, false);
+  if (spec) {
--- End diff --

ZVAL_DEREF checks is reference:

https://github.com/php/php-src/blob/ffc734b2a455e2f2748f18a54ab597f7e0c715ba/Zend/zend_types.h#L944

I removed if statement 


> Extending Thrift class results in "Attempt serialize from non-Thrift object"
> 
>
> Key: THRIFT-4373
> URL: https://issues.apache.org/jira/browse/THRIFT-4373
> Project: Thrift
>  Issue Type: Bug
>  Components: PHP - Library
>Affects Versions: 0.10.0
> Environment: Linux 4.13.3-1-ARCH
> PHP 7.1.10
>Reporter: Josip Sokcevic
> Attachments: 0001-THRIFT-4373-Derefer-PHP-zval-_TSPEC.patch
>
>
> This happens when using php extension. thrift_protocol_write_binary will 
> check Z_TYPE_P of spec and expects to be array (IS_ARRAY). However, PHP7 will 
> set it as reference (IS_REFERENCE).
> Example:
> {code}
> $s = new Serializer\TBinarySerializer();
> // Foo is a Thrift type class
> class FooExtended extends Foo {}
> $o = new Foo();
> $o2 = new FooExtended();
> // this works just fine:
> $s->serialize($o); // this uses thrift_protocol_write_binary if available
> // Next line throws \Thrift\Exception\TProtocolException if 
> thrift_protocol_write_binary is used
> // However, it doesn't break if no extension is available.
> $s->serialize($o);
> {code}
> Proposed solution is to dereference using ZVAL_DEREF before checking types 
> (attached). Alternative would be to mark struct type classes as final, but 
> that break compatibility.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1401: THRIFT-4373: Derefer PHP zval _TSPEC

2017-10-26 Thread sokac
Github user sokac commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1401#discussion_r147295629
  
--- Diff: lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp ---
@@ -537,6 +537,9 @@ void binary_deserialize(int8_t thrift_typeID, 
PHPInputTransport& transport, zval
   }
 
   zval* spec = zend_read_static_property(Z_OBJCE_P(return_value), 
"_TSPEC", sizeof("_TSPEC")-1, false);
+  if (spec) {
--- End diff --

ZVAL_DEREF checks is reference:

https://github.com/php/php-src/blob/ffc734b2a455e2f2748f18a54ab597f7e0c715ba/Zend/zend_types.h#L944

I removed if statement 


---


[jira] [Commented] (THRIFT-4372) Pipe write operations across a network are limited to 65,535 bytes per write.

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4372?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16221324#comment-16221324
 ] 

ASF GitHub Bot commented on THRIFT-4372:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1402
  
Makes sense, thanks for the info.  Every little bit helps us all improve 
what we do next.


> Pipe write operations across a network are limited to 65,535 bytes per write. 
> --
>
> Key: THRIFT-4372
> URL: https://issues.apache.org/jira/browse/THRIFT-4372
> Project: Thrift
>  Issue Type: Bug
>  Components: C# - Library, Delphi - Library
>Reporter: Jens Geyer
>Assignee: Jens Geyer
>Priority: Critical
> Fix For: 0.11.0
>
>
> {quote}Pipe write operations across a network are limited to 65,535 bytes per 
> write. For more information regarding pipes, see the Remarks section.{quote}
> Source: [WriteFileEx 
> function|https://msdn.microsoft.com/en-us/library/windows/desktop/aa365748(v=vs.85).aspx]
> I managed to run into exactly that limit today. Patch follows.
> Symptom is that
>  * the writing end  acts as if it had written all the bytes (in fact, it did)
>  * but the remainder of ~ 65535 bytes is just lost somewhere and never 
> reaches the reading end
> Consequently, the process at the reading end of the pipe gets stuck while 
> waiting for the remaining bytes.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift issue #1402: THRIFT-4372 Pipe write operations across a network are l...

2017-10-26 Thread jeking3
Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1402
  
Makes sense, thanks for the info.  Every little bit helps us all improve 
what we do next.


---


[jira] [Commented] (THRIFT-4372) Pipe write operations across a network are limited to 65,535 bytes per write.

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4372?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16221023#comment-16221023
 ] 

ASF GitHub Bot commented on THRIFT-4372:


Github user Jens-G commented on the issue:

https://github.com/apache/thrift/pull/1402
  
> Why not actually use (2^16)-1 which is the limit?

Several reasons. First, aligned memory access is always faster. If we 
subtract 1 byte, we get the worst case. Next, at least on Windows a number of 
system resources use multiples of 4096, so it is probably not a bad idea to 
follow that model. That's why I picked this particular value.


> Pipe write operations across a network are limited to 65,535 bytes per write. 
> --
>
> Key: THRIFT-4372
> URL: https://issues.apache.org/jira/browse/THRIFT-4372
> Project: Thrift
>  Issue Type: Bug
>  Components: C# - Library, Delphi - Library
>Reporter: Jens Geyer
>Assignee: Jens Geyer
>Priority: Critical
> Fix For: 0.11.0
>
>
> {quote}Pipe write operations across a network are limited to 65,535 bytes per 
> write. For more information regarding pipes, see the Remarks section.{quote}
> Source: [WriteFileEx 
> function|https://msdn.microsoft.com/en-us/library/windows/desktop/aa365748(v=vs.85).aspx]
> I managed to run into exactly that limit today. Patch follows.
> Symptom is that
>  * the writing end  acts as if it had written all the bytes (in fact, it did)
>  * but the remainder of ~ 65535 bytes is just lost somewhere and never 
> reaches the reading end
> Consequently, the process at the reading end of the pipe gets stuck while 
> waiting for the remaining bytes.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift issue #1402: THRIFT-4372 Pipe write operations across a network are l...

2017-10-26 Thread Jens-G
Github user Jens-G commented on the issue:

https://github.com/apache/thrift/pull/1402
  
> Why not actually use (2^16)-1 which is the limit?

Several reasons. First, aligned memory access is always faster. If we 
subtract 1 byte, we get the worst case. Next, at least on Windows a number of 
system resources use multiples of 4096, so it is probably not a bad idea to 
follow that model. That's why I picked this particular value.


---


[jira] [Resolved] (THRIFT-4372) Pipe write operations across a network are limited to 65,535 bytes per write.

2017-10-26 Thread Jens Geyer (JIRA)

 [ 
https://issues.apache.org/jira/browse/THRIFT-4372?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jens Geyer resolved THRIFT-4372.

   Resolution: Fixed
Fix Version/s: 0.11.0

Committed.

> Pipe write operations across a network are limited to 65,535 bytes per write. 
> --
>
> Key: THRIFT-4372
> URL: https://issues.apache.org/jira/browse/THRIFT-4372
> Project: Thrift
>  Issue Type: Bug
>  Components: C# - Library, Delphi - Library
>Reporter: Jens Geyer
>Assignee: Jens Geyer
>Priority: Critical
> Fix For: 0.11.0
>
>
> {quote}Pipe write operations across a network are limited to 65,535 bytes per 
> write. For more information regarding pipes, see the Remarks section.{quote}
> Source: [WriteFileEx 
> function|https://msdn.microsoft.com/en-us/library/windows/desktop/aa365748(v=vs.85).aspx]
> I managed to run into exactly that limit today. Patch follows.
> Symptom is that
>  * the writing end  acts as if it had written all the bytes (in fact, it did)
>  * but the remainder of ~ 65535 bytes is just lost somewhere and never 
> reaches the reading end
> Consequently, the process at the reading end of the pipe gets stuck while 
> waiting for the remaining bytes.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-4372) Pipe write operations across a network are limited to 65,535 bytes per write.

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4372?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16220978#comment-16220978
 ] 

ASF GitHub Bot commented on THRIFT-4372:


Github user asfgit closed the pull request at:

https://github.com/apache/thrift/pull/1402


> Pipe write operations across a network are limited to 65,535 bytes per write. 
> --
>
> Key: THRIFT-4372
> URL: https://issues.apache.org/jira/browse/THRIFT-4372
> Project: Thrift
>  Issue Type: Bug
>  Components: C# - Library, Delphi - Library
>Reporter: Jens Geyer
>Assignee: Jens Geyer
>Priority: Critical
>
> {quote}Pipe write operations across a network are limited to 65,535 bytes per 
> write. For more information regarding pipes, see the Remarks section.{quote}
> Source: [WriteFileEx 
> function|https://msdn.microsoft.com/en-us/library/windows/desktop/aa365748(v=vs.85).aspx]
> I managed to run into exactly that limit today. Patch follows.
> Symptom is that
>  * the writing end  acts as if it had written all the bytes (in fact, it did)
>  * but the remainder of ~ 65535 bytes is just lost somewhere and never 
> reaches the reading end
> Consequently, the process at the reading end of the pipe gets stuck while 
> waiting for the remaining bytes.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1402: THRIFT-4372 Pipe write operations across a networ...

2017-10-26 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/thrift/pull/1402


---


Is there a release schedule?

2017-10-26 Thread Brian Forbis
I’m guessing there isn’t a standard release schedule for thrift which is 
totally fine, but could something be added to the root README to clarify? Even 
something that says “Releases are TBD on an as-needed basis” would be helpful.


[jira] [Commented] (THRIFT-4374) cannot load thrift_protocol due to undefined symbol: _ZTVN10__cxxabiv120__si_class_type_infoE

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4374?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16220680#comment-16220680
 ] 

ASF GitHub Bot commented on THRIFT-4374:


Github user asfgit closed the pull request at:

https://github.com/apache/thrift/pull/1400


> cannot load thrift_protocol due to undefined symbol: 
> _ZTVN10__cxxabiv120__si_class_type_infoE
> -
>
> Key: THRIFT-4374
> URL: https://issues.apache.org/jira/browse/THRIFT-4374
> Project: Thrift
>  Issue Type: Bug
>  Components: PHP - Library
> Environment: PHP 7.1.10, x86_64 GNU/Linux
>Reporter: Robert Lu
>Assignee: James E. King, III
>
> After compiled php extesion, php cannot load php extension:
> at commit 350fe7531feecf7df5208fa19d25730c6ce0a30d
> {code}
> php -d extension=./thrift_protocol.so -m
> PHP Warning: PHP Startup: Unable to load dynamic library 
> './thrift_protocol.so' - ./thrift_protocol.so: undefined symbol: 
> ZTVN10cxxabiv120_si_class_type_infoE in Unknown on line 0
> {code}



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1400: THRIFT-4374 cannot load thrift_protocol due to un...

2017-10-26 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/thrift/pull/1400


---


[jira] [Commented] (THRIFT-4374) cannot load thrift_protocol due to undefined symbol: _ZTVN10__cxxabiv120__si_class_type_infoE

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4374?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16220681#comment-16220681
 ] 

ASF GitHub Bot commented on THRIFT-4374:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1400
  
It only works when creating the PR :)


> cannot load thrift_protocol due to undefined symbol: 
> _ZTVN10__cxxabiv120__si_class_type_infoE
> -
>
> Key: THRIFT-4374
> URL: https://issues.apache.org/jira/browse/THRIFT-4374
> Project: Thrift
>  Issue Type: Bug
>  Components: PHP - Library
> Environment: PHP 7.1.10, x86_64 GNU/Linux
>Reporter: Robert Lu
>Assignee: James E. King, III
>
> After compiled php extesion, php cannot load php extension:
> at commit 350fe7531feecf7df5208fa19d25730c6ce0a30d
> {code}
> php -d extension=./thrift_protocol.so -m
> PHP Warning: PHP Startup: Unable to load dynamic library 
> './thrift_protocol.so' - ./thrift_protocol.so: undefined symbol: 
> ZTVN10cxxabiv120_si_class_type_infoE in Unknown on line 0
> {code}



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift issue #1400: THRIFT-4374 cannot load thrift_protocol due to undefined...

2017-10-26 Thread jeking3
Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1400
  
It only works when creating the PR :)


---


[jira] [Commented] (THRIFT-4374) cannot load thrift_protocol due to undefined symbol: _ZTVN10__cxxabiv120__si_class_type_infoE

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4374?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16220678#comment-16220678
 ] 

ASF GitHub Bot commented on THRIFT-4374:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1400
  
Please remember to put the Jira THRIFT ticket name into the title of every 
pull request.  It makes it easier to merge, as a notification is posted in the 
jira with the pull command to use.


> cannot load thrift_protocol due to undefined symbol: 
> _ZTVN10__cxxabiv120__si_class_type_infoE
> -
>
> Key: THRIFT-4374
> URL: https://issues.apache.org/jira/browse/THRIFT-4374
> Project: Thrift
>  Issue Type: Bug
>  Components: PHP - Library
> Environment: PHP 7.1.10, x86_64 GNU/Linux
>Reporter: Robert Lu
>Assignee: James E. King, III
>
> After compiled php extesion, php cannot load php extension:
> at commit 350fe7531feecf7df5208fa19d25730c6ce0a30d
> {code}
> php -d extension=./thrift_protocol.so -m
> PHP Warning: PHP Startup: Unable to load dynamic library 
> './thrift_protocol.so' - ./thrift_protocol.so: undefined symbol: 
> ZTVN10cxxabiv120_si_class_type_infoE in Unknown on line 0
> {code}



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift issue #1400: THRIFT-4374 add stdc++ back

2017-10-26 Thread jeking3
Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1400
  
Please remember to put the Jira THRIFT ticket name into the title of every 
pull request.  It makes it easier to merge, as a notification is posted in the 
jira with the pull command to use.


---


[jira] [Assigned] (THRIFT-4374) cannot load thrift_protocol due to undefined symbol: _ZTVN10__cxxabiv120__si_class_type_infoE

2017-10-26 Thread James E. King, III (JIRA)

 [ 
https://issues.apache.org/jira/browse/THRIFT-4374?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

James E. King, III reassigned THRIFT-4374:
--

Assignee: James E. King, III  (was: Robert Lu)

> cannot load thrift_protocol due to undefined symbol: 
> _ZTVN10__cxxabiv120__si_class_type_infoE
> -
>
> Key: THRIFT-4374
> URL: https://issues.apache.org/jira/browse/THRIFT-4374
> Project: Thrift
>  Issue Type: Bug
>  Components: PHP - Library
> Environment: PHP 7.1.10, x86_64 GNU/Linux
>Reporter: Robert Lu
>Assignee: James E. King, III
>
> After compiled php extesion, php cannot load php extension:
> at commit 350fe7531feecf7df5208fa19d25730c6ce0a30d
> {code}
> php -d extension=./thrift_protocol.so -m
> PHP Warning: PHP Startup: Unable to load dynamic library 
> './thrift_protocol.so' - ./thrift_protocol.so: undefined symbol: 
> ZTVN10cxxabiv120_si_class_type_infoE in Unknown on line 0
> {code}



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-4374) cannot load thrift_protocol due to undefined symbol: _ZTVN10__cxxabiv120__si_class_type_infoE

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4374?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16220676#comment-16220676
 ] 

ASF GitHub Bot commented on THRIFT-4374:


Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1400#discussion_r147188692
  
--- Diff: lib/php/src/ext/thrift_protocol/config.m4 ---
@@ -25,6 +25,8 @@ PHP_ARG_ENABLE(thrift_protocol, whether to enable the 
thrift_protocol extension,
 
 if test "$PHP_THRIFT_PROTOCOL" != "no"; then
   PHP_REQUIRE_CXX()
+  PHP_ADD_LIBRARY_WITH_PATH(stdc++, "", THRIFT_PROTOCOL_SHARED_LIBADD)
+  PHP_SUBST(THRIFT_PROTOCOL_SHARED_LIBADD)
   CXXFLAGS="$CXXFLAGS -std=c++11"
--- End diff --

Interesting - the project does not require c++11, but apparently here it 
does. :|


> cannot load thrift_protocol due to undefined symbol: 
> _ZTVN10__cxxabiv120__si_class_type_infoE
> -
>
> Key: THRIFT-4374
> URL: https://issues.apache.org/jira/browse/THRIFT-4374
> Project: Thrift
>  Issue Type: Bug
>  Components: PHP - Library
> Environment: PHP 7.1.10, x86_64 GNU/Linux
>Reporter: Robert Lu
>Assignee: Robert Lu
>
> After compiled php extesion, php cannot load php extension:
> at commit 350fe7531feecf7df5208fa19d25730c6ce0a30d
> {code}
> php -d extension=./thrift_protocol.so -m
> PHP Warning: PHP Startup: Unable to load dynamic library 
> './thrift_protocol.so' - ./thrift_protocol.so: undefined symbol: 
> ZTVN10cxxabiv120_si_class_type_infoE in Unknown on line 0
> {code}



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1400: THRIFT-4374 add stdc++ back

2017-10-26 Thread jeking3
Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1400#discussion_r147188692
  
--- Diff: lib/php/src/ext/thrift_protocol/config.m4 ---
@@ -25,6 +25,8 @@ PHP_ARG_ENABLE(thrift_protocol, whether to enable the 
thrift_protocol extension,
 
 if test "$PHP_THRIFT_PROTOCOL" != "no"; then
   PHP_REQUIRE_CXX()
+  PHP_ADD_LIBRARY_WITH_PATH(stdc++, "", THRIFT_PROTOCOL_SHARED_LIBADD)
+  PHP_SUBST(THRIFT_PROTOCOL_SHARED_LIBADD)
   CXXFLAGS="$CXXFLAGS -std=c++11"
--- End diff --

Interesting - the project does not require c++11, but apparently here it 
does. :|


---


[jira] [Commented] (THRIFT-4374) cannot load thrift_protocol due to undefined symbol: _ZTVN10__cxxabiv120__si_class_type_infoE

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4374?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16220663#comment-16220663
 ] 

ASF GitHub Bot commented on THRIFT-4374:


Github user RobberPhex commented on the issue:

https://github.com/apache/thrift/pull/1400
  
There is the [issue](https://issues.apache.org/jira/browse/THRIFT-4374)


> cannot load thrift_protocol due to undefined symbol: 
> _ZTVN10__cxxabiv120__si_class_type_infoE
> -
>
> Key: THRIFT-4374
> URL: https://issues.apache.org/jira/browse/THRIFT-4374
> Project: Thrift
>  Issue Type: Bug
>  Components: PHP - Library
> Environment: PHP 7.1.10, x86_64 GNU/Linux
>Reporter: Robert Lu
>Assignee: Robert Lu
>
> After compiled php extesion, php cannot load php extension:
> at commit 350fe7531feecf7df5208fa19d25730c6ce0a30d
> {code}
> php -d extension=./thrift_protocol.so -m
> PHP Warning: PHP Startup: Unable to load dynamic library 
> './thrift_protocol.so' - ./thrift_protocol.so: undefined symbol: 
> ZTVN10cxxabiv120_si_class_type_infoE in Unknown on line 0
> {code}



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift issue #1400: THRIFT-4374 add stdc++ back

2017-10-26 Thread RobberPhex
Github user RobberPhex commented on the issue:

https://github.com/apache/thrift/pull/1400
  
There is the [issue](https://issues.apache.org/jira/browse/THRIFT-4374)


---


[jira] [Created] (THRIFT-4374) cannot load thrift_protocol due to undefined symbol: _ZTVN10__cxxabiv120__si_class_type_infoE

2017-10-26 Thread Robert Lu (JIRA)
Robert Lu created THRIFT-4374:
-

 Summary: cannot load thrift_protocol due to undefined symbol: 
_ZTVN10__cxxabiv120__si_class_type_infoE
 Key: THRIFT-4374
 URL: https://issues.apache.org/jira/browse/THRIFT-4374
 Project: Thrift
  Issue Type: Bug
  Components: PHP - Library
 Environment: PHP 7.1.10, x86_64 GNU/Linux
Reporter: Robert Lu
Assignee: Robert Lu


After compiled php extesion, php cannot load php extension:

at commit 350fe7531feecf7df5208fa19d25730c6ce0a30d

{code}
php -d extension=./thrift_protocol.so -m
PHP Warning: PHP Startup: Unable to load dynamic library './thrift_protocol.so' 
- ./thrift_protocol.so: undefined symbol: ZTVN10cxxabiv120_si_class_type_infoE 
in Unknown on line 0
{code}




--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-4285) Pull generated send/recv into library to allow behaviour to be customised

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16220658#comment-16220658
 ] 

ASF GitHub Bot commented on THRIFT-4285:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1382
  
0.11.0 cycle hasn't started yet - you still have some time (I don't know 
how much).


> Pull generated send/recv into library to allow behaviour to be customised
> -
>
> Key: THRIFT-4285
> URL: https://issues.apache.org/jira/browse/THRIFT-4285
> Project: Thrift
>  Issue Type: Improvement
>  Components: Go - Compiler, Go - Library
>Reporter: Chris Bannister
>Assignee: Chris Bannister
> Attachments: 0001-go-pull-generated-send-recv-into-lib-v6.patch, 
> 0001-go-pull-generated-send-recv-into-lib-v7.patch
>
>
> Currently it is difficult to change how thrift writes messages onto the 
> transport because they are in the generated code. Instead the generated 
> send/recv methods should be in the library. This will greatly simplify the 
> client code and remove many duplicate methods whilst allowing users more 
> flexibility to implement connection pools and other features such as THeader.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-4372) Pipe write operations across a network are limited to 65,535 bytes per write.

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4372?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16220653#comment-16220653
 ] 

ASF GitHub Bot commented on THRIFT-4372:


Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1402#discussion_r147185064
  
--- Diff: lib/csharp/src/Transport/TNamedPipeClientTransport.cs ---
@@ -88,7 +89,18 @@ public override void Write(byte[] buf, int off, int len)
 throw new 
TTransportException(TTransportException.ExceptionType.NotOpen);
 }
 
-client.Write(buf, off, len);
+// if necessary, send the data in chunks
+// there's a system limit around 0x1 bytes that we hit 
otherwise
+// MSDN: "Pipe write operations across a network are limited 
to 65,535 bytes per write. For more information regarding pipes, see the 
Remarks section."
+var nBytes = Math.Min(len, 15 * 4096);  // 16 would exceed the 
limit
--- End diff --

Why not actually use `(2^16)-1` which is the limit?


> Pipe write operations across a network are limited to 65,535 bytes per write. 
> --
>
> Key: THRIFT-4372
> URL: https://issues.apache.org/jira/browse/THRIFT-4372
> Project: Thrift
>  Issue Type: Bug
>  Components: C# - Library, Delphi - Library
>Reporter: Jens Geyer
>Assignee: Jens Geyer
>Priority: Critical
>
> {quote}Pipe write operations across a network are limited to 65,535 bytes per 
> write. For more information regarding pipes, see the Remarks section.{quote}
> Source: [WriteFileEx 
> function|https://msdn.microsoft.com/en-us/library/windows/desktop/aa365748(v=vs.85).aspx]
> I managed to run into exactly that limit today. Patch follows.
> Symptom is that
>  * the writing end  acts as if it had written all the bytes (in fact, it did)
>  * but the remainder of ~ 65535 bytes is just lost somewhere and never 
> reaches the reading end
> Consequently, the process at the reading end of the pipe gets stuck while 
> waiting for the remaining bytes.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-4372) Pipe write operations across a network are limited to 65,535 bytes per write.

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4372?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16220655#comment-16220655
 ] 

ASF GitHub Bot commented on THRIFT-4372:


Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1402#discussion_r147185494
  
--- Diff: lib/csharp/src/Transport/TNamedPipeServerTransport.cs ---
@@ -239,40 +239,51 @@ public override void Write(byte[] buf, int off, int 
len)
 throw new 
TTransportException(TTransportException.ExceptionType.NotOpen);
 }
 
-if (asyncMode)
+// if necessary, send the data in chunks
+// there's a system limit around 0x1 bytes that we hit 
otherwise
+// MSDN: "Pipe write operations across a network are 
limited to 65,535 bytes per write. For more information regarding pipes, see 
the Remarks section."
+var nBytes = Math.Min(len, 15 * 4096);  // 16 would exceed 
the limit
--- End diff --

Why not actually use (2^16)-1 which is the limit?


> Pipe write operations across a network are limited to 65,535 bytes per write. 
> --
>
> Key: THRIFT-4372
> URL: https://issues.apache.org/jira/browse/THRIFT-4372
> Project: Thrift
>  Issue Type: Bug
>  Components: C# - Library, Delphi - Library
>Reporter: Jens Geyer
>Assignee: Jens Geyer
>Priority: Critical
>
> {quote}Pipe write operations across a network are limited to 65,535 bytes per 
> write. For more information regarding pipes, see the Remarks section.{quote}
> Source: [WriteFileEx 
> function|https://msdn.microsoft.com/en-us/library/windows/desktop/aa365748(v=vs.85).aspx]
> I managed to run into exactly that limit today. Patch follows.
> Symptom is that
>  * the writing end  acts as if it had written all the bytes (in fact, it did)
>  * but the remainder of ~ 65535 bytes is just lost somewhere and never 
> reaches the reading end
> Consequently, the process at the reading end of the pipe gets stuck while 
> waiting for the remaining bytes.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-4372) Pipe write operations across a network are limited to 65,535 bytes per write.

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4372?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16220656#comment-16220656
 ] 

ASF GitHub Bot commented on THRIFT-4372:


Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1402#discussion_r147185326
  
--- Diff: lib/csharp/src/Transport/TNamedPipeClientTransport.cs ---
@@ -88,7 +89,18 @@ public override void Write(byte[] buf, int off, int len)
 throw new 
TTransportException(TTransportException.ExceptionType.NotOpen);
 }
 
-client.Write(buf, off, len);
+// if necessary, send the data in chunks
+// there's a system limit around 0x1 bytes that we hit 
otherwise
+// MSDN: "Pipe write operations across a network are limited 
to 65,535 bytes per write. For more information regarding pipes, see the 
Remarks section."
+var nBytes = Math.Min(len, 15 * 4096);  // 16 would exceed the 
limit
+while (nBytes > 0)
+{
+client.Write(buf, off, nBytes);
+
+off += nBytes;
+len -= nBytes;
+nBytes = Math.Min(len, nBytes);
--- End diff --

Shouldn't this be the same calculation as the one outside the loop?  
Technically it doesn't have to be, however I found this confusing to read.


> Pipe write operations across a network are limited to 65,535 bytes per write. 
> --
>
> Key: THRIFT-4372
> URL: https://issues.apache.org/jira/browse/THRIFT-4372
> Project: Thrift
>  Issue Type: Bug
>  Components: C# - Library, Delphi - Library
>Reporter: Jens Geyer
>Assignee: Jens Geyer
>Priority: Critical
>
> {quote}Pipe write operations across a network are limited to 65,535 bytes per 
> write. For more information regarding pipes, see the Remarks section.{quote}
> Source: [WriteFileEx 
> function|https://msdn.microsoft.com/en-us/library/windows/desktop/aa365748(v=vs.85).aspx]
> I managed to run into exactly that limit today. Patch follows.
> Symptom is that
>  * the writing end  acts as if it had written all the bytes (in fact, it did)
>  * but the remainder of ~ 65535 bytes is just lost somewhere and never 
> reaches the reading end
> Consequently, the process at the reading end of the pipe gets stuck while 
> waiting for the remaining bytes.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-4372) Pipe write operations across a network are limited to 65,535 bytes per write.

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4372?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16220654#comment-16220654
 ] 

ASF GitHub Bot commented on THRIFT-4372:


Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1402#discussion_r147185736
  
--- Diff: lib/csharp/src/Transport/TNamedPipeServerTransport.cs ---
@@ -239,40 +239,51 @@ public override void Write(byte[] buf, int off, int 
len)
 throw new 
TTransportException(TTransportException.ExceptionType.NotOpen);
 }
 
-if (asyncMode)
+// if necessary, send the data in chunks
+// there's a system limit around 0x1 bytes that we hit 
otherwise
+// MSDN: "Pipe write operations across a network are 
limited to 65,535 bytes per write. For more information regarding pipes, see 
the Remarks section."
+var nBytes = Math.Min(len, 15 * 4096);  // 16 would exceed 
the limit
+while (nBytes > 0)
 {
-Exception eOuter = null;
-var evt = new ManualResetEvent(false);
 
-stream.BeginWrite(buf, off, len, asyncResult =>
+if (asyncMode)
 {
-try
-{
-if (stream != null)
-stream.EndWrite(asyncResult);
-else
-eOuter = new 
TTransportException(TTransportException.ExceptionType.Interrupted);
-}
-catch (Exception e)
-{
-if (stream != null)
-eOuter = e;
-else
-eOuter = new 
TTransportException(TTransportException.ExceptionType.Interrupted, e.Message);
-}
-evt.Set();
-}, null);
+Exception eOuter = null;
+var evt = new ManualResetEvent(false);
 
-evt.WaitOne();
+stream.BeginWrite(buf, off, nBytes, asyncResult =>
+{
+try
+{
+if (stream != null)
+stream.EndWrite(asyncResult);
+else
+eOuter = new 
TTransportException(TTransportException.ExceptionType.Interrupted);
+}
+catch (Exception e)
+{
+if (stream != null)
+eOuter = e;
+else
+eOuter = new 
TTransportException(TTransportException.ExceptionType.Interrupted, e.Message);
+}
+evt.Set();
+}, null);
+
+evt.WaitOne();
+
+if (eOuter != null)
+throw eOuter; // rethrow exception
+}
+else
+{
+stream.Write(buf, off, nBytes);
+}
 
-if (eOuter != null)
-throw eOuter; // rethrow exception
+off += nBytes;
+len -= nBytes;
+nBytes = Math.Min(len, nBytes);
--- End diff --

Shouldn't this be the same calculation as the one outside the loop?  
Technically it doesn't have to be, however I found this confusing to read.


> Pipe write operations across a network are limited to 65,535 bytes per write. 
> --
>
> Key: THRIFT-4372
> URL: https://issues.apache.org/jira/browse/THRIFT-4372
> Project: Thrift
>  Issue Type: Bug
>  Components: C# - Library, Delphi - Library
>Reporter: Jens Geyer
>Assignee: Jens Geyer
>Priority: Critical
>
> {quote}Pipe write operations across a network are limited to 65,535 bytes per 
> write. For more information regarding pipes, see the Remarks section.{quote}
> Source: [WriteFileEx 
> function|https://msdn.microsoft.com/en-us/library/windows/desktop/aa365748(v=vs.85).aspx]
> I managed to run into 

[GitHub] thrift issue #1382: THRIFT-4285 Move TX/RX methods from gen. code to library

2017-10-26 Thread jeking3
Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1382
  
0.11.0 cycle hasn't started yet - you still have some time (I don't know 
how much).


---


[GitHub] thrift pull request #1402: THRIFT-4372 Pipe write operations across a networ...

2017-10-26 Thread jeking3
Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1402#discussion_r147185494
  
--- Diff: lib/csharp/src/Transport/TNamedPipeServerTransport.cs ---
@@ -239,40 +239,51 @@ public override void Write(byte[] buf, int off, int 
len)
 throw new 
TTransportException(TTransportException.ExceptionType.NotOpen);
 }
 
-if (asyncMode)
+// if necessary, send the data in chunks
+// there's a system limit around 0x1 bytes that we hit 
otherwise
+// MSDN: "Pipe write operations across a network are 
limited to 65,535 bytes per write. For more information regarding pipes, see 
the Remarks section."
+var nBytes = Math.Min(len, 15 * 4096);  // 16 would exceed 
the limit
--- End diff --

Why not actually use (2^16)-1 which is the limit?


---


[GitHub] thrift pull request #1402: THRIFT-4372 Pipe write operations across a networ...

2017-10-26 Thread jeking3
Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1402#discussion_r147185064
  
--- Diff: lib/csharp/src/Transport/TNamedPipeClientTransport.cs ---
@@ -88,7 +89,18 @@ public override void Write(byte[] buf, int off, int len)
 throw new 
TTransportException(TTransportException.ExceptionType.NotOpen);
 }
 
-client.Write(buf, off, len);
+// if necessary, send the data in chunks
+// there's a system limit around 0x1 bytes that we hit 
otherwise
+// MSDN: "Pipe write operations across a network are limited 
to 65,535 bytes per write. For more information regarding pipes, see the 
Remarks section."
+var nBytes = Math.Min(len, 15 * 4096);  // 16 would exceed the 
limit
--- End diff --

Why not actually use `(2^16)-1` which is the limit?


---


[GitHub] thrift pull request #1402: THRIFT-4372 Pipe write operations across a networ...

2017-10-26 Thread jeking3
Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1402#discussion_r147185326
  
--- Diff: lib/csharp/src/Transport/TNamedPipeClientTransport.cs ---
@@ -88,7 +89,18 @@ public override void Write(byte[] buf, int off, int len)
 throw new 
TTransportException(TTransportException.ExceptionType.NotOpen);
 }
 
-client.Write(buf, off, len);
+// if necessary, send the data in chunks
+// there's a system limit around 0x1 bytes that we hit 
otherwise
+// MSDN: "Pipe write operations across a network are limited 
to 65,535 bytes per write. For more information regarding pipes, see the 
Remarks section."
+var nBytes = Math.Min(len, 15 * 4096);  // 16 would exceed the 
limit
+while (nBytes > 0)
+{
+client.Write(buf, off, nBytes);
+
+off += nBytes;
+len -= nBytes;
+nBytes = Math.Min(len, nBytes);
--- End diff --

Shouldn't this be the same calculation as the one outside the loop?  
Technically it doesn't have to be, however I found this confusing to read.


---


[GitHub] thrift pull request #1402: THRIFT-4372 Pipe write operations across a networ...

2017-10-26 Thread jeking3
Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1402#discussion_r147185736
  
--- Diff: lib/csharp/src/Transport/TNamedPipeServerTransport.cs ---
@@ -239,40 +239,51 @@ public override void Write(byte[] buf, int off, int 
len)
 throw new 
TTransportException(TTransportException.ExceptionType.NotOpen);
 }
 
-if (asyncMode)
+// if necessary, send the data in chunks
+// there's a system limit around 0x1 bytes that we hit 
otherwise
+// MSDN: "Pipe write operations across a network are 
limited to 65,535 bytes per write. For more information regarding pipes, see 
the Remarks section."
+var nBytes = Math.Min(len, 15 * 4096);  // 16 would exceed 
the limit
+while (nBytes > 0)
 {
-Exception eOuter = null;
-var evt = new ManualResetEvent(false);
 
-stream.BeginWrite(buf, off, len, asyncResult =>
+if (asyncMode)
 {
-try
-{
-if (stream != null)
-stream.EndWrite(asyncResult);
-else
-eOuter = new 
TTransportException(TTransportException.ExceptionType.Interrupted);
-}
-catch (Exception e)
-{
-if (stream != null)
-eOuter = e;
-else
-eOuter = new 
TTransportException(TTransportException.ExceptionType.Interrupted, e.Message);
-}
-evt.Set();
-}, null);
+Exception eOuter = null;
+var evt = new ManualResetEvent(false);
 
-evt.WaitOne();
+stream.BeginWrite(buf, off, nBytes, asyncResult =>
+{
+try
+{
+if (stream != null)
+stream.EndWrite(asyncResult);
+else
+eOuter = new 
TTransportException(TTransportException.ExceptionType.Interrupted);
+}
+catch (Exception e)
+{
+if (stream != null)
+eOuter = e;
+else
+eOuter = new 
TTransportException(TTransportException.ExceptionType.Interrupted, e.Message);
+}
+evt.Set();
+}, null);
+
+evt.WaitOne();
+
+if (eOuter != null)
+throw eOuter; // rethrow exception
+}
+else
+{
+stream.Write(buf, off, nBytes);
+}
 
-if (eOuter != null)
-throw eOuter; // rethrow exception
+off += nBytes;
+len -= nBytes;
+nBytes = Math.Min(len, nBytes);
--- End diff --

Shouldn't this be the same calculation as the one outside the loop?  
Technically it doesn't have to be, however I found this confusing to read.


---


[GitHub] thrift issue #1400: add stdc++ back

2017-10-26 Thread jeking3
Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1400
  
What's the Jira ticket number this is associated with?


---


[jira] [Commented] (THRIFT-2998) Node.js: Missing header from http request

2017-10-26 Thread James E. King, III (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-2998?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16220634#comment-16220634
 ] 

James E. King, III commented on THRIFT-2998:


While this change looks good, when I enable cross testing of nodejs and http 
transport, some things pass and some do not.  Too many do not...  *grumble*

> Node.js: Missing header from http request
> -
>
> Key: THRIFT-2998
> URL: https://issues.apache.org/jira/browse/THRIFT-2998
> Project: Thrift
>  Issue Type: Bug
>  Components: Node.js - Library
>Affects Versions: 0.9.2
>Reporter: Traian Stanev
>Assignee: James E. King, III
>Priority: Minor
>
> The "Content-Type" header is not being set to "application/x-thrift" when 
> making http requests in the node.js implementation. This results in error 
> response (status 400) for any such connections (depending on whether the 
> server checks the header). The Python implementation for example adds the 
> header explicitly:
> https://github.com/tstanev/thrift/blob/master/lib/py/src/transport/THttpClient.py#L127



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Updated] (THRIFT-4370) Ubuntu Artful cppcheck and flake8 are more stringent and causing SCA build job failures

2017-10-26 Thread James E. King, III (JIRA)

 [ 
https://issues.apache.org/jira/browse/THRIFT-4370?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

James E. King, III updated THRIFT-4370:
---
Priority: Blocker  (was: Minor)

> Ubuntu Artful cppcheck and flake8 are more stringent and causing SCA build 
> job failures
> ---
>
> Key: THRIFT-4370
> URL: https://issues.apache.org/jira/browse/THRIFT-4370
> Project: Thrift
>  Issue Type: Bug
>  Components: Build Process
>Affects Versions: 0.11.0
> Environment: docker ubuntu-artful
>Reporter: James E. King, III
>Assignee: James E. King, III
>Priority: Blocker
> Fix For: 0.11.0
>
>




--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Resolved] (THRIFT-4370) Ubuntu Artful cppcheck and flake8 are more stringent and causing SCA build job failures

2017-10-26 Thread James E. King, III (JIRA)

 [ 
https://issues.apache.org/jira/browse/THRIFT-4370?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

James E. King, III resolved THRIFT-4370.

   Resolution: Fixed
Fix Version/s: 0.11.0

> Ubuntu Artful cppcheck and flake8 are more stringent and causing SCA build 
> job failures
> ---
>
> Key: THRIFT-4370
> URL: https://issues.apache.org/jira/browse/THRIFT-4370
> Project: Thrift
>  Issue Type: Bug
>  Components: Build Process
>Affects Versions: 0.11.0
> Environment: docker ubuntu-artful
>Reporter: James E. King, III
>Assignee: James E. King, III
>Priority: Blocker
> Fix For: 0.11.0
>
>




--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-4370) Ubuntu Artful cppcheck and flake8 are more stringent and causing SCA build job failures

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16220632#comment-16220632
 ] 

ASF GitHub Bot commented on THRIFT-4370:


Github user asfgit closed the pull request at:

https://github.com/apache/thrift/pull/1399


> Ubuntu Artful cppcheck and flake8 are more stringent and causing SCA build 
> job failures
> ---
>
> Key: THRIFT-4370
> URL: https://issues.apache.org/jira/browse/THRIFT-4370
> Project: Thrift
>  Issue Type: Bug
>  Components: Build Process
>Affects Versions: 0.11.0
> Environment: docker ubuntu-artful
>Reporter: James E. King, III
>Assignee: James E. King, III
>Priority: Blocker
> Fix For: 0.11.0
>
>




--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1399: THRIFT-4370: build generated code before running ...

2017-10-26 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/thrift/pull/1399


---


[jira] [Commented] (THRIFT-4370) Ubuntu Artful cppcheck and flake8 are more stringent and causing SCA build job failures

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16220523#comment-16220523
 ] 

ASF GitHub Bot commented on THRIFT-4370:


Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1399#discussion_r147157619
  
--- Diff: lib/py/setup.py ---
@@ -22,7 +22,7 @@
 import sys
 try:
 from setuptools import setup, Extension
-except:
+except Exception:
--- End diff --

I have to resubmit anyway to kick off another PR build, but I'd like to 
leave it as I changed it for now, and it can be improved later through a 
subsequent PR.  I was just trying to eliminate those pesky E722 errors in 
flake8.  There are a lot more issues in generated code that flake8 pulls out.


> Ubuntu Artful cppcheck and flake8 are more stringent and causing SCA build 
> job failures
> ---
>
> Key: THRIFT-4370
> URL: https://issues.apache.org/jira/browse/THRIFT-4370
> Project: Thrift
>  Issue Type: Bug
>  Components: Build Process
>Affects Versions: 0.11.0
> Environment: docker ubuntu-artful
>Reporter: James E. King, III
>Assignee: James E. King, III
>Priority: Blocker
>




--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1399: THRIFT-4370: build generated code before running ...

2017-10-26 Thread jeking3
Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1399#discussion_r147157619
  
--- Diff: lib/py/setup.py ---
@@ -22,7 +22,7 @@
 import sys
 try:
 from setuptools import setup, Extension
-except:
+except Exception:
--- End diff --

I have to resubmit anyway to kick off another PR build, but I'd like to 
leave it as I changed it for now, and it can be improved later through a 
subsequent PR.  I was just trying to eliminate those pesky E722 errors in 
flake8.  There are a lot more issues in generated code that flake8 pulls out.


---


[jira] [Commented] (THRIFT-4373) Extending Thrift class results in "Attempt serialize from non-Thrift object"

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4373?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16220023#comment-16220023
 ] 

ASF GitHub Bot commented on THRIFT-4373:


Github user RobberPhex commented on the issue:

https://github.com/apache/thrift/pull/1401
  
If we want process REFERENCE everywhere.
I think we should review every line of `zval` used, is it?


> Extending Thrift class results in "Attempt serialize from non-Thrift object"
> 
>
> Key: THRIFT-4373
> URL: https://issues.apache.org/jira/browse/THRIFT-4373
> Project: Thrift
>  Issue Type: Bug
>  Components: PHP - Library
>Affects Versions: 0.10.0
> Environment: Linux 4.13.3-1-ARCH
> PHP 7.1.10
>Reporter: Josip Sokcevic
> Attachments: 0001-THRIFT-4373-Derefer-PHP-zval-_TSPEC.patch
>
>
> This happens when using php extension. thrift_protocol_write_binary will 
> check Z_TYPE_P of spec and expects to be array (IS_ARRAY). However, PHP7 will 
> set it as reference (IS_REFERENCE).
> Example:
> {code}
> $s = new Serializer\TBinarySerializer();
> // Foo is a Thrift type class
> class FooExtended extends Foo {}
> $o = new Foo();
> $o2 = new FooExtended();
> // this works just fine:
> $s->serialize($o); // this uses thrift_protocol_write_binary if available
> // Next line throws \Thrift\Exception\TProtocolException if 
> thrift_protocol_write_binary is used
> // However, it doesn't break if no extension is available.
> $s->serialize($o);
> {code}
> Proposed solution is to dereference using ZVAL_DEREF before checking types 
> (attached). Alternative would be to mark struct type classes as final, but 
> that break compatibility.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift issue #1401: THRIFT-4373: Derefer PHP zval _TSPEC

2017-10-26 Thread RobberPhex
Github user RobberPhex commented on the issue:

https://github.com/apache/thrift/pull/1401
  
If we want process REFERENCE everywhere.
I think we should review every line of `zval` used, is it?


---


[jira] [Commented] (THRIFT-4373) Extending Thrift class results in "Attempt serialize from non-Thrift object"

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4373?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16220018#comment-16220018
 ] 

ASF GitHub Bot commented on THRIFT-4373:


Github user RobberPhex commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1401#discussion_r147051880
  
--- Diff: lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp ---
@@ -537,6 +537,9 @@ void binary_deserialize(int8_t thrift_typeID, 
PHPInputTransport& transport, zval
   }
 
   zval* spec = zend_read_static_property(Z_OBJCE_P(return_value), 
"_TSPEC", sizeof("_TSPEC")-1, false);
+  if (spec) {
--- End diff --

After `zend_read_static_property` with `slient=false`, we should detect 
exception.
If no exception, we needn't detect spec is null.

but we need detect that `Z_TYPE_P(prop) == IS_REFERENCE`


> Extending Thrift class results in "Attempt serialize from non-Thrift object"
> 
>
> Key: THRIFT-4373
> URL: https://issues.apache.org/jira/browse/THRIFT-4373
> Project: Thrift
>  Issue Type: Bug
>  Components: PHP - Library
>Affects Versions: 0.10.0
> Environment: Linux 4.13.3-1-ARCH
> PHP 7.1.10
>Reporter: Josip Sokcevic
> Attachments: 0001-THRIFT-4373-Derefer-PHP-zval-_TSPEC.patch
>
>
> This happens when using php extension. thrift_protocol_write_binary will 
> check Z_TYPE_P of spec and expects to be array (IS_ARRAY). However, PHP7 will 
> set it as reference (IS_REFERENCE).
> Example:
> {code}
> $s = new Serializer\TBinarySerializer();
> // Foo is a Thrift type class
> class FooExtended extends Foo {}
> $o = new Foo();
> $o2 = new FooExtended();
> // this works just fine:
> $s->serialize($o); // this uses thrift_protocol_write_binary if available
> // Next line throws \Thrift\Exception\TProtocolException if 
> thrift_protocol_write_binary is used
> // However, it doesn't break if no extension is available.
> $s->serialize($o);
> {code}
> Proposed solution is to dereference using ZVAL_DEREF before checking types 
> (attached). Alternative would be to mark struct type classes as final, but 
> that break compatibility.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-4373) Extending Thrift class results in "Attempt serialize from non-Thrift object"

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4373?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16220019#comment-16220019
 ] 

ASF GitHub Bot commented on THRIFT-4373:


Github user RobberPhex commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1401#discussion_r147052024
  
--- Diff: lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp ---
@@ -699,6 +702,9 @@ void binary_serialize_hashtable_key(int8_t keytype, 
PHPOutputTransport& transpor
 
 static
 void binary_serialize(int8_t thrift_typeID, PHPOutputTransport& transport, 
zval* value, HashTable* fieldspec) {
+  if (value) {
--- End diff --

`Z_TYPE_P(prop) == IS_REFERENCE`


> Extending Thrift class results in "Attempt serialize from non-Thrift object"
> 
>
> Key: THRIFT-4373
> URL: https://issues.apache.org/jira/browse/THRIFT-4373
> Project: Thrift
>  Issue Type: Bug
>  Components: PHP - Library
>Affects Versions: 0.10.0
> Environment: Linux 4.13.3-1-ARCH
> PHP 7.1.10
>Reporter: Josip Sokcevic
> Attachments: 0001-THRIFT-4373-Derefer-PHP-zval-_TSPEC.patch
>
>
> This happens when using php extension. thrift_protocol_write_binary will 
> check Z_TYPE_P of spec and expects to be array (IS_ARRAY). However, PHP7 will 
> set it as reference (IS_REFERENCE).
> Example:
> {code}
> $s = new Serializer\TBinarySerializer();
> // Foo is a Thrift type class
> class FooExtended extends Foo {}
> $o = new Foo();
> $o2 = new FooExtended();
> // this works just fine:
> $s->serialize($o); // this uses thrift_protocol_write_binary if available
> // Next line throws \Thrift\Exception\TProtocolException if 
> thrift_protocol_write_binary is used
> // However, it doesn't break if no extension is available.
> $s->serialize($o);
> {code}
> Proposed solution is to dereference using ZVAL_DEREF before checking types 
> (attached). Alternative would be to mark struct type classes as final, but 
> that break compatibility.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-4373) Extending Thrift class results in "Attempt serialize from non-Thrift object"

2017-10-26 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4373?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16220020#comment-16220020
 ] 

ASF GitHub Bot commented on THRIFT-4373:


Github user RobberPhex commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1401#discussion_r147052177
  
--- Diff: lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp ---
@@ -709,6 +715,9 @@ void binary_serialize(int8_t thrift_typeID, 
PHPOutputTransport& transport, zval*
 throw_tprotocolexception("Attempt to send non-object type as a 
T_STRUCT", INVALID_DATA);
   }
   zval* spec = zend_read_static_property(Z_OBJCE_P(value), "_TSPEC", 
sizeof("_TSPEC")-1, true);
+  if (spec) {
--- End diff --

I think `Z_TYPE_P(prop) == IS_REFERENCE`


> Extending Thrift class results in "Attempt serialize from non-Thrift object"
> 
>
> Key: THRIFT-4373
> URL: https://issues.apache.org/jira/browse/THRIFT-4373
> Project: Thrift
>  Issue Type: Bug
>  Components: PHP - Library
>Affects Versions: 0.10.0
> Environment: Linux 4.13.3-1-ARCH
> PHP 7.1.10
>Reporter: Josip Sokcevic
> Attachments: 0001-THRIFT-4373-Derefer-PHP-zval-_TSPEC.patch
>
>
> This happens when using php extension. thrift_protocol_write_binary will 
> check Z_TYPE_P of spec and expects to be array (IS_ARRAY). However, PHP7 will 
> set it as reference (IS_REFERENCE).
> Example:
> {code}
> $s = new Serializer\TBinarySerializer();
> // Foo is a Thrift type class
> class FooExtended extends Foo {}
> $o = new Foo();
> $o2 = new FooExtended();
> // this works just fine:
> $s->serialize($o); // this uses thrift_protocol_write_binary if available
> // Next line throws \Thrift\Exception\TProtocolException if 
> thrift_protocol_write_binary is used
> // However, it doesn't break if no extension is available.
> $s->serialize($o);
> {code}
> Proposed solution is to dereference using ZVAL_DEREF before checking types 
> (attached). Alternative would be to mark struct type classes as final, but 
> that break compatibility.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1401: THRIFT-4373: Derefer PHP zval _TSPEC

2017-10-26 Thread RobberPhex
Github user RobberPhex commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1401#discussion_r147051880
  
--- Diff: lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp ---
@@ -537,6 +537,9 @@ void binary_deserialize(int8_t thrift_typeID, 
PHPInputTransport& transport, zval
   }
 
   zval* spec = zend_read_static_property(Z_OBJCE_P(return_value), 
"_TSPEC", sizeof("_TSPEC")-1, false);
+  if (spec) {
--- End diff --

After `zend_read_static_property` with `slient=false`, we should detect 
exception.
If no exception, we needn't detect spec is null.

but we need detect that `Z_TYPE_P(prop) == IS_REFERENCE`


---


[GitHub] thrift pull request #1401: THRIFT-4373: Derefer PHP zval _TSPEC

2017-10-26 Thread RobberPhex
Github user RobberPhex commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1401#discussion_r147052024
  
--- Diff: lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp ---
@@ -699,6 +702,9 @@ void binary_serialize_hashtable_key(int8_t keytype, 
PHPOutputTransport& transpor
 
 static
 void binary_serialize(int8_t thrift_typeID, PHPOutputTransport& transport, 
zval* value, HashTable* fieldspec) {
+  if (value) {
--- End diff --

`Z_TYPE_P(prop) == IS_REFERENCE`


---


[GitHub] thrift pull request #1401: THRIFT-4373: Derefer PHP zval _TSPEC

2017-10-26 Thread RobberPhex
Github user RobberPhex commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1401#discussion_r147052177
  
--- Diff: lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp ---
@@ -709,6 +715,9 @@ void binary_serialize(int8_t thrift_typeID, 
PHPOutputTransport& transport, zval*
 throw_tprotocolexception("Attempt to send non-object type as a 
T_STRUCT", INVALID_DATA);
   }
   zval* spec = zend_read_static_property(Z_OBJCE_P(value), "_TSPEC", 
sizeof("_TSPEC")-1, true);
+  if (spec) {
--- End diff --

I think `Z_TYPE_P(prop) == IS_REFERENCE`


---