The user-declared move-assignment operator for ConflictSet overwrites impl without destroying the existing implementation object, leaking all memory and resources owned by the left-hand side. It also mishandles self-move-assignment by setting impl to nullptr, leaving the object in an invalid state.
Locations:
ConflictSet.cpp:5591-5594
SkipList.cpp:983-986
HashTable.cpp:121-124
All three implementations share the same definition:
The move constructor is fine because a newly constructed object has no existing impl to destroy, but the move-assignment operator must first call internal_destroy(impl) (or the equivalent per-implementation cleanup) before taking ownership of other.impl.
Impact:
cs1 = std::move(cs2); leaks whatever tree/heap state cs1 previously owned.
cs = std::move(cs); sets impl to nullptr, so the destructor does not run and the existing state is leaked.
Expected behavior:
Move-assignment should clean up the old implementation and be self-assignment-safe, e.g.:
(Each implementation would use its own destroy helper instead of internal_destroy.)
Minimal reproducer:
#include"ConflictSet.h"#include<utility>intmain(){weaselab::ConflictSeta(0);a.addWrites(nullptr,0,1);// allocate some internal state
weaselab::ConflictSetb(1);a=std::move(b);// leaks a's original impl
// a = std::move(a); // also leaks and leaves a.impl == nullptr
}
The user-declared move-assignment operator for `ConflictSet` overwrites `impl` without destroying the existing implementation object, leaking all memory and resources owned by the left-hand side. It also mishandles self-move-assignment by setting `impl` to `nullptr`, leaving the object in an invalid state.
**Locations:**
- `ConflictSet.cpp:5591-5594`
- `SkipList.cpp:983-986`
- `HashTable.cpp:121-124`
All three implementations share the same definition:
```cpp
ConflictSet &ConflictSet::operator=(ConflictSet &&other) noexcept {
impl = std::exchange(other.impl, nullptr);
return *this;
}
```
The move constructor is fine because a newly constructed object has no existing `impl` to destroy, but the move-assignment operator must first call `internal_destroy(impl)` (or the equivalent per-implementation cleanup) before taking ownership of `other.impl`.
**Impact:**
- `cs1 = std::move(cs2);` leaks whatever tree/heap state `cs1` previously owned.
- `cs = std::move(cs);` sets `impl` to `nullptr`, so the destructor does not run and the existing state is leaked.
**Expected behavior:**
Move-assignment should clean up the old implementation and be self-assignment-safe, e.g.:
```cpp
ConflictSet &ConflictSet::operator=(ConflictSet &&other) noexcept {
if (this != &other) {
if (impl) {
internal_destroy(impl);
}
impl = std::exchange(other.impl, nullptr);
}
return *this;
}
```
(Each implementation would use its own destroy helper instead of `internal_destroy`.)
**Minimal reproducer:**
```cpp
#include "ConflictSet.h"
#include <utility>
int main() {
weaselab::ConflictSet a(0);
a.addWrites(nullptr, 0, 1); // allocate some internal state
weaselab::ConflictSet b(1);
a = std::move(b); // leaks a's original impl
// a = std::move(a); // also leaks and leaves a.impl == nullptr
}
```
weaselbot
was assigned by andrew2026-06-21 23:34:29 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
The user-declared move-assignment operator for
ConflictSetoverwritesimplwithout destroying the existing implementation object, leaking all memory and resources owned by the left-hand side. It also mishandles self-move-assignment by settingimpltonullptr, leaving the object in an invalid state.Locations:
ConflictSet.cpp:5591-5594SkipList.cpp:983-986HashTable.cpp:121-124All three implementations share the same definition:
The move constructor is fine because a newly constructed object has no existing
implto destroy, but the move-assignment operator must first callinternal_destroy(impl)(or the equivalent per-implementation cleanup) before taking ownership ofother.impl.Impact:
cs1 = std::move(cs2);leaks whatever tree/heap statecs1previously owned.cs = std::move(cs);setsimpltonullptr, so the destructor does not run and the existing state is leaked.Expected behavior:
Move-assignment should clean up the old implementation and be self-assignment-safe, e.g.:
(Each implementation would use its own destroy helper instead of
internal_destroy.)Minimal reproducer:
Fixed in #55.