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

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

[~emmenlau] Yes, there is a workaround by making sure you never typedef an 
exception if you have C++ clients, so in that sense it is not urgent. But, this 
knowledge was gained after many days of debugging since it's very opaque as to 
why things crash with access violation errors and it was a complete showstopper 
for us once we hit it. There is no documentation/guide that says you shouldn't 
typedef exceptions, so there could be others who have encountered this. I don't 
know if this was fixed after 0.11, but it is there in 0.11 for sure.
As a side note, I no longer use Thrift, so unless I can get it to build 
locally, I won't be able to verify a potential fix.


was (Author: andrejn):
[~emmenlau] Yes, there is a workaround by making sure you never typedef an 
exception if you have C++ clients, so in that sense it is not urgent. But, this 
knowledge was gained after many days of debugging since it's very opaque as to 
why things crash with access violation errors and it was complete showstopper 
for us once we hit it. There is no documentation/guide that says you shouldn't 
typedef exceptions, so there could be others who have encountered this. I don't 
know if this was fixed after 0.11, but it is there in 0.11 for sure.
As a side note, I no longer use Thrift, so unless I can get it to build 
locally, I won't be able to verify a potential fix.

> 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)

Reply via email to