[jira] [Comment Edited] (THRIFT-4781) C++ clients crash when exceptions are typedefed in the IDL

["Andrej Nazarov (Jira)" <ji...@apache.org>]

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

Andrej Nazarov edited comment on THRIFT-4781 at 5/6/20, 1:47 PM:
-----------------------------------------------------------------

Hello [~emmenlau]!

Maybe this thread is getting a bit hijacked. I admit partial responsibility for that since I listed problems in getting Thrift to build for me. With that being said:

1) Maybe it's worth creating a separate JIRA for this discussion (building Thrift)? I personally tried creating the project files using CMakeGUI, and you can see in my first comment that I got an error in the Configure stage with BoostMacros, so I couldn't proceed. Not sure what [~Xusheng Zhang] experience was in that sense.

2) [~Xusheng Zhang]: -You can try removing the files from the libthrift.vcxproj file (e.g. remove [this line|https://github.com/apache/thrift/blob/0.13.0/lib/cpp/libthrift.vcxproj#L40] and a few after it). I removed them and the build didn't complain about it.- Looking at your screenshots removing reference to these doesn't work. Not sure then why they're in the proj file if they aren't anywhere in the repo...

[~zeshuai007] wrote that he couldn't reproduce the error, I've asked for reproduction code he used so I can check.

[~emmenlau] - can you try perhaps to reproduce the actual problem in the JIRA? It is pretty easy:

1) Create a simple exception struct in one Thrift schema file
 2) Create a service in another Thrift schema file that throws this exception, but make sure to typedef the exception itself in the schema file.
 3) Generate a service (in any language) that throws this exception
 4) Generate a C++ client and have it try to call the service and get the response back
 For me it was 100% reproducible that things blew up when you tried to call a service that threw a typedeffed exception from the C++ client. The client always died and it was working when the exception typedef was removed in the Thrift schema file.


was (Author: andrejn):
Hello [~emmenlau]!

Maybe this thread is getting a bit hijacked. I admit partial responsibility for that since I listed problems in getting Thrift to build for me. With that being said:

1) Maybe it's worth creating a separate JIRA for this discussion (building Thrift)? I personally tried creating the project files using CMakeGUI, and you can see in my first comment that I got an error in the Configure stage with BoostMacros, so I couldn't proceed. Not sure what [~Xusheng Zhang] experience was in that sense.

2) [~Xusheng Zhang]: You can try removing the files from the libthrift.vcxproj file (e.g. remove [this line|https://github.com/apache/thrift/blob/0.13.0/lib/cpp/libthrift.vcxproj#L40] and a few after it). I removed them and the build didn't complain about it.

[~zeshuai007] wrote that he couldn't reproduce the error, I've asked for reproduction code he used so I can check.

[~emmenlau] - can you try perhaps to reproduce the actual problem in the JIRA? It is pretty easy:

1) Create a simple exception struct in one Thrift schema file
 2) Create a service in another Thrift schema file that throws this exception, but make sure to typedef the exception itself in the schema file.
 3) Generate a service (in any language) that throws this exception
 4) Generate a C++ client and have it try to call the service and get the response back
 For me it was 100% reproducible that things blew up when you tried to call a service that threw a typedeffed exception from the C++ client. The client always died and it was working when the exception typedef was removed in the Thrift schema file.

> C++ clients crash when exceptions are typedefed in the IDL
> ----------------------------------------------------------
>
>                 Key: THRIFT-4781
>                 URL: https://issues.apache.org/jira/browse/THRIFT-4781
>             Project: Thrift
>          Issue Type: Bug
>          Components: C++ - Compiler
>    Affects Versions: 0.11.0, 0.12.0
>            Reporter: Andrej Nazarov
>            Priority: Major
>         Attachments: image-2020-05-06-20-15-44-986.png, image-2020-05-06-21-37-46-809.png
>
>
> If exceptions are typedefed in the IDL, they're generated as pointers in Cpp. This causes a runtime crash (memory access violation) on the C++ client-side when a server sends that exception and the client tries to read it. Example follows:
> {code:java|title=service.thrift}
> namespace * thrifttest.service
> include "errors.thrift"
> typedef errors.FooError FooError
> service FooBarService
> {
> 	string getFooString(1: i32 stringLength) throws (1: FooError e);
> 	string getBarString(1: i32 stringLength) throws (1: errors.BarError e);
> }
> {code}
> {code:java|title=errors.thrift}
> namespace * thrifttest.errors
> exception FooError {
>   1: string message
> }
> exception BarError {
>   1: string message
> }
> {code}
> {code:java|title=FooBarService.h}
> class FooBarService_getFooString_presult {
>  public:
>   virtual ~FooBarService_getFooString_presult() throw();
>   std::string* success;
>   FooError* e; //note pointer declaration of the exception field
> // snip...
> class FooBarService_getBarString_presult {
>  public:
>   virtual ~FooBarService_getBarString_presult() throw();
>   std::string* success;
>    ::thrifttest::errors::BarError e; //note different declaration of the exception field
> //snip
> {code}
> {code:java|title=FooBarService.cpp}
> uint32_t FooBarService_getFooString_presult::read(::apache::thrift::protocol::TProtocol* iprot) {
> // snip...
>   while (true)
>   {
> // snip...
>     switch (fid)
>     {
> // snip...
>       case 1:
>         if (ftype == ::apache::thrift::protocol::T_STRUCT) {
>           xfer += (*(this->e)).read(iprot); // <-- this line causes access violation crash because the pointer is not initialized
>           this->__isset.e = true;
>        // snip...
> uint32_t FooBarService_getBarString_presult::read(::apache::thrift::protocol::TProtocol* iprot) {
> // snip...
>   while (true)
>   {
> // snip...
>     switch (fid)
>     {
> // snip...
>       case 1:
>         if (ftype == ::apache::thrift::protocol::T_STRUCT) {
>           xfer += this->e.read(iprot); //<-- this gets read OK.
>           this->__isset.e = true;
> //snip
> {code}
> This happens regardless of server language (reproducible if server throwing the exceptions is Java, Python or C++)
>  I guess this logic in [t_cpp_generator.cc:1104|https://github.com/apache/thrift/blob/0.11.0/compiler/cpp/src/thrift/generate/t_cpp_generator.cc#L1104] gets deceived in case of typedefed exceptions:
> {code:java|title=t_cpp_generator.cc}
> (pointers && !(*m_iter)->get_type()->is_xception()),
> {code}
> I'm no Thrift compiler expert, but I assume there is a reason why you don't want exceptions to be declared as pointers. Yet in this case they clearly are.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)