Skip to content

Commit

Permalink
fix: correct reference counting for Slice
Browse files Browse the repository at this point in the history
  • Loading branch information
aceforeverd committed Nov 11, 2024
1 parent e6796f0 commit 43df8d5
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 26 deletions.
18 changes: 13 additions & 5 deletions hybridse/include/base/fe_slice.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <string.h>

#include <string>
#include <utility>

#include "base/raw_buffer.h"

Expand Down Expand Up @@ -100,6 +101,12 @@ class Slice {
return ((size_ >= x.size_) && (memcmp(data_, x.data_, x.size_) == 0));
}

protected:
void _swap(Slice &slice) {
std::swap(data_, slice.data_);
std::swap(size_, slice.size_);
}

private:
uint32_t size_;
const char *data_;
Expand Down Expand Up @@ -154,16 +161,17 @@ class RefCountedSlice : public Slice {
private:
RefCountedSlice(int8_t *data, size_t size, bool managed)
: Slice(reinterpret_cast<const char *>(data), size),
ref_cnt_(managed ? new int(1) : nullptr) {}
ref_cnt_(managed ? new std::atomic<int32_t>(1) : nullptr) {}

RefCountedSlice(const char *data, size_t size, bool managed)
: Slice(data, size), ref_cnt_(managed ? new int(1) : nullptr) {}
: Slice(data, size), ref_cnt_(managed ? new std::atomic<int32_t>(1) : nullptr) {}

void Release();
void _release();

void Update(const RefCountedSlice &slice);
void _copy(const RefCountedSlice &slice);
void _swap(RefCountedSlice &slice);

int32_t *ref_cnt_;
std::atomic<int32_t> *ref_cnt_;
};

} // namespace base
Expand Down
40 changes: 19 additions & 21 deletions hybridse/src/base/fe_slice.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,56 +15,54 @@
*/

#include "base/fe_slice.h"
#include <atomic>

namespace hybridse {
namespace base {

RefCountedSlice::~RefCountedSlice() { Release(); }
RefCountedSlice::~RefCountedSlice() { _release(); }

void RefCountedSlice::Release() {
void RefCountedSlice::_release() {
if (this->ref_cnt_ != nullptr) {
auto& cnt = *this->ref_cnt_;
cnt -= 1;
if (cnt == 0 && buf() != nullptr) {
// memset in case the buf is still used after free
memset(buf(), 0, size());
if (std::atomic_fetch_add_explicit(ref_cnt_, -1, std::memory_order_release) == 1) {
std::atomic_thread_fence(std::memory_order_acquire);
assert(buf());
free(buf());
delete this->ref_cnt_;
}
}
}

void RefCountedSlice::Update(const RefCountedSlice& slice) {
void RefCountedSlice::_copy(const RefCountedSlice& slice) {
reset(slice.data(), slice.size());
this->ref_cnt_ = slice.ref_cnt_;
ref_cnt_ = slice.ref_cnt_;
if (this->ref_cnt_ != nullptr) {
(*this->ref_cnt_) += 1;
std::atomic_fetch_add_explicit(ref_cnt_, 1, std::memory_order_relaxed);
}
}

RefCountedSlice::RefCountedSlice(const RefCountedSlice& slice) {
this->Update(slice);
void RefCountedSlice::_swap(RefCountedSlice& slice) {
std::swap(ref_cnt_, slice.ref_cnt_);
Slice::_swap(slice);
}

RefCountedSlice::RefCountedSlice(RefCountedSlice&& slice) {
this->Update(slice);
RefCountedSlice::RefCountedSlice(const RefCountedSlice& slice) {
_copy(slice);
}

RefCountedSlice::RefCountedSlice(RefCountedSlice&& slice) { _swap(slice); }

Check warning on line 53 in hybridse/src/base/fe_slice.cc

View check run for this annotation

Codecov / codecov/patch

hybridse/src/base/fe_slice.cc#L53

Added line #L53 was not covered by tests

RefCountedSlice& RefCountedSlice::operator=(const RefCountedSlice& slice) {
if (&slice == this) {
return *this;
}
this->Release();
this->Update(slice);
_release();
_copy(slice);
return *this;
}

RefCountedSlice& RefCountedSlice::operator=(RefCountedSlice&& slice) {
if (&slice == this) {
return *this;
}
this->Release();
this->Update(slice);
_swap(slice);
return *this;
}

Expand Down

0 comments on commit 43df8d5

Please sign in to comment.