fix: replace manual destructor calls in constructors with explicit cleanup

[JoeSergen]

Bug Report

Affected Code

Three constructors call this->~ClassName() from catch blocks — 5 call sites across 3 files:

File	Constructor	Line	Method
so/ring-buffer.cpp	RingBuffer(int map_fd, ...)	116	BPF ring buffer
so/ring-buffer.cpp	RingBuffer(size_t bsz)	131, 142	Normal ring buffer (2 sites)
so/bpf-manager.cpp	BPFManager()	174	BPF manager
so/data-map.cpp	DataMap()	60	Data map

Root Cause

Calling a destructor from a constructor is fragile for two reasons:

1. Maintenance risk: if the destructor is later modified to use members that haven't been initialized yet at the point of the catch, undefined behavior follows.
2. Concrete crash bug in RingBuffer Normal constructor: type = RING_BUF_TYPE_NORMAL is set after the first try block, so the destructor takes the BPF cleanup path on uninitialized data.

RingBuffer Normal Constructor (the crash case)

The destructor uses type to decide which cleanup path to take:

RingBuffer::~RingBuffer()
{
    if (type == RING_BUF_TYPE_NORMAL)   // line 152
    {
        // Normal path: delete heap objects (null-safe)
        if (spinlock)   { delete spinlock; }
        if (mirror_shm) { delete mirror_shm; }
        if (shm_ctl)    { delete shm_ctl; }
    }
    else  // type == 0 (RING_BUF_TYPE_BPF)
    {
        // BPF path: munmap and close on raw pointers
        if ((ulong)comsumer_index > 0)    // line 174 — checks garbage!
            munmap((void *)comsumer_index, page_size);
        if ((ulong)producer_index > 0)    // line 179 — checks garbage!
            munmap((void *)producer_index, page_size + 2 * bsz);
        if (epoll_fd > 0)                 // line 184 — checks garbage!
            close(epoll_fd);
    }
}

But in the Normal constructor, type is assigned too late:

RingBuffer::RingBuffer(size_t bsz) : mirror_shm(nullptr)  // line 120
{
    try
    {
        shm_ctl = new SharedMemory();           // could throw
        spinlock = new SpinLock(...);            // could throw ← BUG TRIGGER
        comsumer_index = &shm_ctl->rdi;          // pointer assignment
        producer_index = &shm_ctl->wri;          // pointer assignment
    }
    catch (...)
    {
        this->~RingBuffer();  // type is still 0! Takes BPF path!
        throw;
    }
    type = RING_BUF_TYPE_NORMAL;  // ← assigned AFTER the try block
    // ...
}

When new SpinLock() throws after new SharedMemory() succeeded:

• shm_ctl is a valid heap pointer
• spinlock is null (from = nullptr in the union)
• comsumer_index points to a field inside shm_ctl's shared memory (valid pointer, but not mmap'd)
• type is still 0 (RING_BUF_TYPE_BPF)
• The destructor enters the BPF path, evaluates (ulong)comsumer_index > 0 → TRUE (it's a valid kernel shared-memory pointer, not NULL)
• Calls munmap(comsumer_index, page_size) on a kernel pointer → crash with SIGSEGV

Fix

Replace each this->~ClassName() with explicit cleanup of only the members that were successfully allocated. This approach:

1. Cleans up exactly what was allocated, in the correct order
2. Removes dependency on the destructor being null-safe
3. Uses SAFE_DELETE (the project's established pattern from include/com.h:432) and proper MAP_FAILED checks instead of fragile (ulong)ptr > 0

1. ring-buffer.cpp BPF constructor (line 116)

Before:

err_out:
    this->~RingBuffer();
    throw exc;

After:

err_out:
    if (producer_index && producer_index != MAP_FAILED)
    {
        munmap((void *)producer_index, page_size + 2 * bsz);
        producer_index = NULL;
    }
    if (comsumer_index && comsumer_index != MAP_FAILED)
    {
        munmap((void *)comsumer_index, page_size);
        comsumer_index = NULL;
    }
    if (epoll_fd >= 0)
    {
        close(epoll_fd);
        epoll_fd = -1;
    }
    throw exc;

comsumer_index and producer_index are set via mmap() with explicit NULL / MAP_FAILED checks in the BPF constructor (lines 71-72, 83-84, 91-92), so the cleanup logic is exactly aligned with the initialization logic.

2. ring-buffer.cpp Normal constructor (lines 120-148)

Before:

RingBuffer::RingBuffer(size_t bsz) : mirror_shm(nullptr)
{
    try
    {
        shm_ctl = new SharedMemory();
        spinlock = new SpinLock(&shm_ctl->ring_buffer_lock);
        comsumer_index = &shm_ctl->rdi;
        producer_index = &shm_ctl->wri;
    }
    catch (...)
    {
        this->~RingBuffer();  // ← type is still 0, BPF path! CRASH!
        throw;
    }
    type = RING_BUF_TYPE_NORMAL;  // ← too late!
    key_t key = 0x12345678 + bsz;
    try
    {
        mirror_shm = new MirrorMemory(bsz, key);
    }
    catch (...)
    {
        this->~RingBuffer();
        throw;
    }
    this->data = mirror_shm->getaddr();
    this->bsz = bsz;
    return;
}

After:

RingBuffer::RingBuffer(size_t bsz) : mirror_shm(nullptr)
{
    type = RING_BUF_TYPE_NORMAL;  // ← moved BEFORE the try block
    try
    {
        shm_ctl = new SharedMemory();
        spinlock = new SpinLock(&shm_ctl->ring_buffer_lock);
        comsumer_index = &shm_ctl->rdi;
        producer_index = &shm_ctl->wri;
    }
    catch (...)
    {
        SAFE_DELETE(spinlock);
        SAFE_DELETE(shm_ctl);
        throw;
    }
    key_t key = 0x12345678 + bsz;
    try
    {
        mirror_shm = new MirrorMemory(bsz, key);
    }
    catch (...)
    {
        SAFE_DELETE(spinlock);
        SAFE_DELETE(shm_ctl);
        throw;
    }
    this->data = mirror_shm->getaddr();
    this->bsz = bsz;
    return;
}

Key changes:

• type moved before the first try block — fixes the crash bug
• Both catch blocks use SAFE_DELETE on only the heap-allocated members (spinlock, shm_ctl)
• comsumer_index and producer_index are pointer assignments to fields inside shm_ctl, not independently allocated, so they need no cleanup
• mirror_shm is only allocated in the second try block, so it only exists if the second catch fires — but by then type is already set correctly

3. bpf-manager.cpp (line 174)

Before:

BPFManager::BPFManager()
{
    int err = 0;
    try
    {
        m_shm = new SharedMemory();
        m_bpf_lock = new SpinLock(&m_shm->bpf_lock);
        bpf_ref_cnt = &m_shm->bpf_ref_cnt;
    }
    catch (...)
    {
        this->~BPFManager();
        throw;
    }
    m_bpf_lock->lock();
    // ...

After:

BPFManager::BPFManager()
{
    int err = 0;
    try
    {
        m_shm = new SharedMemory();
        m_bpf_lock = new SpinLock(&m_shm->bpf_lock);
        bpf_ref_cnt = &m_shm->bpf_ref_cnt;
    }
    catch (...)
    {
        SAFE_DELETE(m_bpf_lock);
        SAFE_DELETE(m_shm);
        throw;
    }
    m_bpf_lock->lock();
    // ...

bpf_ref_cnt is a pointer to a field inside m_shm, not independently allocated — no cleanup needed.

4. data-map.cpp (line 60)

Before:

DataMap::DataMap()
{
    // ... size calculations ...
    try
    {
        m_shm = new SharedMemory();
        m_bpf = new BPFManager();
        m_rb = new RingBuffer(ent_sz);
        m_lock = new SpinLock(&m_shm->data_map_lock);
        m_entrys = (typeof(m_entrys))m_rb->buf();
        m_idx = &m_shm->data_map_idx;
        m_bpf_rb = new RingBuffer(m_bpf->m_map_fd, handle_event, this);
    }
    catch (...)
    {
        this->~DataMap();
        throw;
    }
    // ...

After:

DataMap::DataMap()
{
    // ... size calculations ...
    try
    {
        m_shm = new SharedMemory();
        m_bpf = new BPFManager();
        m_rb = new RingBuffer(ent_sz);
        m_lock = new SpinLock(&m_shm->data_map_lock);
        m_entrys = (typeof(m_entrys))m_rb->buf();
        m_idx = &m_shm->data_map_idx;
        m_bpf_rb = new RingBuffer(m_bpf->m_map_fd, handle_event, this);
    }
    catch (...)
    {
        SAFE_DELETE(m_bpf_rb);
        SAFE_DELETE(m_lock);
        SAFE_DELETE(m_rb);
        SAFE_DELETE(m_bpf);
        SAFE_DELETE(m_shm);
        throw;
    }
    // ...

Cleanup order is reverse of allocation order — matching how the original destructor would release them. m_entrys and m_idx are pointer assignments to shared memory fields, not heap allocations.