thrift-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andrej Nazarov (Jira)" <j...@apache.org>
Subject [jira] [Comment Edited] (THRIFT-4781) C++ clients crash when exceptions are typedefed in the IDL
Date Wed, 06 May 2020 13:58:00 GMT

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

Mime
View raw message