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

Christopher Friedt updated THRIFT-5678:
---------------------------------------
    Description: 
In TConnectedClient.h the destructor is non-virtual, which causes the warning 
below.
{code:java}
In member function 'void 
apache::thrift::server::TServerFramework::disposeConnectedClient(apache::thrift::server::TConnectedClient*)':
lib/cpp/src/thrift/server/TServerFramework.cpp:234:3: warning: deleting object 
of polymorphic class type 'apache::thrift::server::TConnectedClient' which has 
non-virtual destructor might cause undefined behavior 
[-Wdelete-non-virtual-dtor]
  234 |   delete pClient;
      |   ^~~~~~~~~~~~~~{code}
Not itself a major problem, but Zephyr CI promotes all warnings to errors, 
which causes builds to fail.

TConnectedClient.cpp has the following line of code:
{code:java}
TConnectedClient::~TConnectedClient() = default;{code}
It might be considered a regression. The commit that removed the virtual 
qualifier was 042580f53441efe1bc5c80c89351fcb30740659e

Suggested fix is to mark it as virtual and set it to default in the header, and 
then delete the definition in the .cpp file.

Alternatively, keep the .cpp dtor, but simply re-add the virtual keyword in the 
header

  was:
In TConnectedClient.h the destructor is non-virtual, which causes the warning 
below.
{code:java}
In member function 'void 
apache::thrift::server::TServerFramework::disposeConnectedClient(apache::thrift::server::TConnectedClient*)':
lib/cpp/src/thrift/server/TServerFramework.cpp:234:3: warning: deleting object 
of polymorphic class type 'apache::thrift::server::TConnectedClient' which has 
non-virtual destructor might cause undefined behavior 
[-Wdelete-non-virtual-dtor]
  234 |   delete pClient;
      |   ^~~~~~~~~~~~~~{code}

Not itself a major problem, but Zephyr CI promotes all warnings to errors, 
which causes builds to fail.

TConnectedClient.cpp has the following line of code:
{code:java}
TConnectedClient::~TConnectedClient() = default;{code}

It might be considered a regression. The commit that removed the virtual 
qualifier was 042580f53441efe1bc5c80c89351fcb30740659e

Suggested fix is to mark it as virtual and set it to default in the header, and 
then delete the definition in the .cpp file.


> lib: cpp: TConnectedClient: warning due to non-virtual dtor
> -----------------------------------------------------------
>
>                 Key: THRIFT-5678
>                 URL: https://issues.apache.org/jira/browse/THRIFT-5678
>             Project: Thrift
>          Issue Type: Bug
>          Components: C++ - Library
>    Affects Versions: 0.13.0, 0.14.0, 0.15.0, 0.14.1, 0.14.2, 0.16.0, 0.17.0
>            Reporter: Christopher Friedt
>            Priority: Minor
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> In TConnectedClient.h the destructor is non-virtual, which causes the warning 
> below.
> {code:java}
> In member function 'void 
> apache::thrift::server::TServerFramework::disposeConnectedClient(apache::thrift::server::TConnectedClient*)':
> lib/cpp/src/thrift/server/TServerFramework.cpp:234:3: warning: deleting 
> object of polymorphic class type 'apache::thrift::server::TConnectedClient' 
> which has non-virtual destructor might cause undefined behavior 
> [-Wdelete-non-virtual-dtor]
>   234 |   delete pClient;
>       |   ^~~~~~~~~~~~~~{code}
> Not itself a major problem, but Zephyr CI promotes all warnings to errors, 
> which causes builds to fail.
> TConnectedClient.cpp has the following line of code:
> {code:java}
> TConnectedClient::~TConnectedClient() = default;{code}
> It might be considered a regression. The commit that removed the virtual 
> qualifier was 042580f53441efe1bc5c80c89351fcb30740659e
> Suggested fix is to mark it as virtual and set it to default in the header, 
> and then delete the definition in the .cpp file.
> Alternatively, keep the .cpp dtor, but simply re-add the virtual keyword in 
> the header



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to