[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #1119: Support more `ZRANGE` options

[GitBox <gi...@apache.org>]

PragmaTwice commented on PR #1119:
URL: https://github.com/apache/incubator-kvrocks/pull/1119#issuecomment-1315472652

   I want to explain why kvrocks crashes when @tanruixiang is using `unique_ptr`.
   
   Firstly, he creates `ZrangeCommon` and some derived structs of it, like this:
   ```
   struct ZrangeCommon { ... };
   struct ZrangeLex : ZrangeCommon { std::string x };
   struct ZrangeIndex : ZrangeCommon { ... };
   ```
   
   Then, he use `unique_ptr` to store `ZrangeCommon`:
   ```
   unique_ptr<ZrangeCommon> spec_;
   
   ...
   
   if (...) {
     spec_ = make_unique<ZrangeLex>(...);
   } else if (...) {
     spec_ = make_unique<ZrangeIndex>(...);
   }
   ```
   
   Note: Unlike `shared_ptr`, `unique_ptr` does not do type erasure, which means that the default deleter in `unique_ptr` is determined at compile time for the destructor. In this case, destructor of `ZrangeCommon` will be called when `spec_` is destructed. Note that the actual object type at this point may be `ZRangeLex`, and calling the wrong destructor may not only lead to resource leaks, but also to other catastrophic consequences due to UB.
   
   The design of `shared_ptr` is different in that `shared_ptr` does type erasure and its deleter is stored in runtime, which makes it store the exact function pointer of the constructore at construction (which, of course, adds runtime overhead), so there is no such problem.
   
   So a better solution is to use a virtual destructor: `shared_ptr` is not needed here, because `shared_ptr` has extra overhead and usefulness:
   
   ```
   struct ZrangeCommon {
     ...
     virtual ~ZrangeCommon() {}
   };
   ```
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org