When the RL::Interface is destroied but the engine isn't completely shoutdown there was a dangling pointer. The duration the dangling pointer is accessable got expanded with rP27908.
Whith this patch there no dangling pointer (in that regard).
Details
- Reviewers
vladislavbelov - Trac Tickets
- #5989
Run with the rl-interface.
Restart the engine while the rl-interface runs (from the mod selector). The rl-interface should also restart
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Lint Skipped - Unit
Unit Tests Skipped - Build Status
Buildable 22861 Build 56104: Vulcan Build Build 56103: Vulcan Build (macOS) Build 56102: Vulcan Build (Windows)
Event Timeline
source/rlinterface/RLInterface.h | ||
---|---|---|
167 | Maybe keep the mg in the variable name? Also why a unique pointer instead of just using the destructor to clean up? |
source/rlinterface/RLInterface.h | ||
---|---|---|
167 | Adding a "mg" feels redundant. Propably something like httpServerContext. https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rr-ptr |
source/rlinterface/RLInterface.h | ||
---|---|---|
167 | Here the pointer is private and no copy can be obtained, therefore the Interface already serves as "unique pointer" or in other words is unmistakably the owner. Basically this feels like double wrapping. |
source/rlinterface/RLInterface.h | ||
---|---|---|
167 | Externaly yes. But inside of RL::Interface member functions thous things are possible. Stoping the context is the responsibility of the object. (If I would redesign mongoose in C++ you wouldn't have to manually stop the context.) I'd like to use the destructor only for things where you need multiple member variables. https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-dtor-ptr |
source/rlinterface/RLInterface.h | ||
---|---|---|
167 | Well, guarding against internal misuses seems somewhat futile an undertaking in the first place, at least I find increasing size to achieve it questionable. Guess would be nice if one or two more people could voice there take. |
source/rlinterface/RLInterface.h | ||
---|---|---|
167 | This patch is there to solve an internal misuse. |
source/rlinterface/RLInterface.h | ||
---|---|---|
167 | Source size obviously but that can be ignored for all intent and purpose. Binary size should increase while the worst case of size on the stack/heap with how you crafted it (empty deleter) should have been avoided. Guess the binary size increase could be more than you expect. |
source/rlinterface/RLInterface.h | ||
---|---|---|
167 | Since std::default_delete is an empty class implementations should optimize std::unique_ptr for empty deleter. |