[SERVER-56061] Mark MaterializedRow move constructor and destructor as noexcept Created: 13/Apr/21 Updated: 29/Oct/23 Resolved: 19/Apr/21 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 5.0.0-rc0 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Nikita Lapkov (Inactive) | Assignee: | Nikita Lapkov (Inactive) |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Backwards Compatibility: | Fully Compatible |
| Operating System: | ALL |
| Sprint: | Query Execution 2021-04-19, Query Execution 2021-05-03 |
| Participants: |
| Description |
|
The repro below shows how accessor, pointing to the element of spool buffer, can become invalid after new element is added to the spool buffer.
This was discovered during manual inspection of values in the following SBE plan:
After lspool stage added a new element to the spool buffer, reallocation of buffer happened and value behind s7 became invalid. This happened because std::vector has strong exception guarantees for push_back method. Section 26.3.11.5 of n4659 C++17 standard says that *If an exception is thrown while inserting a single element at the end and T is CopyInsertable or is_nothrow_move_constructible_v<T> is true, there are no effects.* We know that type MaterializedRow is CopyInsertable, so std::vector::push_back must guarantee that even if exception happens during the insertion, there are no effects. We also know that is_nothrow_move_constructible_v<MaterializedRow> is false, meaning that move constructor of MaterializedRow can potentially throw. So, the implementation of std::vector::push_back cannot use it because if this constructor will throw, object from which we move can be in an inconsistent state, which is an effect. As a result, std::vector::push_back uses copy constructor of MaterializedRow. This forces MaterializedRow to delete old values and create new ones, making accessor to the old values invalid. To avoid this problem, we can mark MaterializedRow's move constructor and destructor as noexcept. After this change, is_nothrow_move_constructible_v<MaterializedRow> becomes true and move constructor is used by std::vector::push_back. |
| Comments |
| Comment by Githook User [ 19/Apr/21 ] |
|
Author: {'name': 'Nikita Lapkov', 'email': 'nikita.lapkov@mongodb.com', 'username': 'laplab'}Message: |