fix: replace manual destructor calls in constructors with explicit cleanup

[JoeSergen]

Summary

Replace this->~ClassName() calls in constructor catch blocks with explicit member cleanup. 5 call sites across 3 files.

Root Cause

The Pattern (all 3 constructors)

Calling a destructor from a constructor is fragile: if the destructor is later modified to use members that haven't been initialized yet, undefined behavior follows.

The Concrete Crash Bug (RingBuffer Normal constructor)

In the Normal constructor, type = RING_BUF_TYPE_NORMAL was set after the first try block. The destructor dispatches on type:

// Destructor — line 150
RingBuffer::~RingBuffer()
{
    if (type == RING_BUF_TYPE_NORMAL)
    {
        if (spinlock)   { delete spinlock; }    // null-safe
        if (mirror_shm) { delete mirror_shm; }
        if (shm_ctl)    { delete shm_ctl; }
    }
    else  // type == 0 (BPF) — WRONG PATH taken before this fix
    {
        if ((ulong)comsumer_index > 0)   // garbage pointer dereference
            munmap((void *)comsumer_index, page_size);
        if ((ulong)producer_index > 0)   // garbage
            munmap((void *)producer_index, page_size + 2 * bsz);
        if (epoll_fd > 0)                // garbage
            close(epoll_fd);
    }
}

Before this fix:

RingBuffer::RingBuffer(size_t bsz) : mirror_shm(nullptr)
{
    try { ... }
    catch (...) {
        this->~RingBuffer();  // type is still 0 → takes BPF path → CRASH
        throw;
    }
    type = RING_BUF_TYPE_NORMAL;  // assigned too late!
    try { ... }
    catch (...) { this->~RingBuffer(); }  // type is now 3, OK
}

If new SharedMemory() succeeds but new SpinLock() fails, comsumer_index points to a valid shared-memory address. (ulong)comsumer_index > 0 evaluates to TRUE, and munmap() is called on a kernel pointer → SIGSEGV.

Changes

1. ring-buffer.cpp — BPF constructor err_out label

Replaced this->~RingBuffer() with explicit cleanup using proper MAP_FAILED checks (instead of fragile (ulong)ptr > 0):

err_out:
    if (producer_index && producer_index != MAP_FAILED) { munmap(...); }
    if (comsumer_index && comsumer_index != MAP_FAILED) { munmap(...); }
    if (epoll_fd >= 0)                                  { close(epoll_fd); }
    throw exc;

2. ring-buffer.cpp — Normal constructor

• Moved type = RING_BUF_TYPE_NORMAL; before the first try block (fixes the crash)
• Both catch blocks use SAFE_DELETE on heap-allocated members only

RingBuffer::RingBuffer(size_t bsz) : mirror_shm(nullptr)
{
    type = RING_BUF_TYPE_NORMAL;  // ← moved here
    try {
        shm_ctl = new SharedMemory();
        spinlock = new SpinLock(&shm_ctl->ring_buffer_lock);
        ...
    } catch (...) {
        SAFE_DELETE(spinlock);
        SAFE_DELETE(shm_ctl);
        throw;
    }
    try {
        mirror_shm = new MirrorMemory(bsz, key);
    } catch (...) {
        SAFE_DELETE(spinlock);
        SAFE_DELETE(shm_ctl);
        throw;
    }
    ...
}

3. bpf-manager.cpp

catch (...) {
    SAFE_DELETE(m_bpf_lock);
    SAFE_DELETE(m_shm);
    throw;
}

4. data-map.cpp

catch (...) {
    SAFE_DELETE(m_bpf_rb);   // reverse init order
    SAFE_DELETE(m_lock);
    SAFE_DELETE(m_rb);
    SAFE_DELETE(m_bpf);
    SAFE_DELETE(m_shm);
    throw;
}

Non-heap Members

These members are pointer assignments into shared memory — they point to fields inside heap-allocated objects, not independently allocated memory. They are NOT freed by cleanup:

• bpf_ref_cnt → &m_shm->bpf_ref_cnt
• comsumer_index → &shm_ctl->rdi
• producer_index → &shm_ctl->wri
• m_entrys → m_rb->buf()
• m_idx → &m_shm->data_map_idx

Verification

• SAFE_DELETE is the project's established pattern (include/com.h:432)
• Only heap-allocated members are cleaned up
• Cleanup order is reverse of allocation order
• No ABI or header changes

Closes #113

[dkapture-ci-bot]

你好，这是对 libdkapture#114 fix: replace manual destructor calls in constructors with ex 的评审。

本轮为 DKapture 两仓（libdkapture / dkapture-bpf）全部以 fix 开头 open PR 的批量评审，共 17 个；汇总表如下，本 PR 加粗。逐项详评见分隔线下方。

PR	标题	作者	规模	结论	主要风险
#149	fix(net-traffic): initialize rules before loadin	yuKing123-king	+19/-9	可合并	规则安装顺序与其它工具约定不一致；本仓改动无正确性缺陷
#147	fix(net-filter): abort rule loading on invalid c	yuKing123-king	+9/-3	需修改	失败时 clear_rules() 波及 add_rule() 装入的规则；空白注释行导致整个加载失败
#141	fix(dkapture): honor parsed pid in read(vector<p	JoeSergen	+23/-2	需修改	unsafe_find 单记录语义使 /proc//fd 只回调一次；返回值语义漂移
#140	fix(dkapture): return bytes read, not remaining	JoeSergen	+10/-4	可合并	无阻塞风险；与文档契约对齐，仓内无调用方依赖旧语义
#114	fix: replace manual destructor calls in construc	JoeSergen	+28/-7	需修改	捆绑了与 open PR #112 逐字节相同的 fs_watch 修复，跨 PR 重复
#125	fix(lsof): start ringbuf consumer before iterato	yuKing123-king	+17/-3	需修改	线程启动后错误路径仍 goto err_out → 释放运行中 rb（UAF）+ 线程泄漏
#112	fix: fs_watch calls trace_file_init instead of m	JoeSergen	+1/-1	可合并	无；仅修一处复制粘贴错误
#103	fix: make power-snoop internal symbols static fo	yuKing123-king	+3/-2	可合并	改动无害；但 PR 描述的 multiple definition 在当前构建配置下无法复现
#119	fix(syscall-stat): stop skipping syscall key 0 d	yuKing123-king	+18/-6	可合并	循环终止四条路径已逐一核实；缩进与提交拆分小问题
#115	fix(syscall-stat): improve builtin flow	yuKing123-king	+204/-41	需修改	三处 bpf_get_map_fd 错误路径未设 ret，最终 return ret 误报成功
#117	fix(trace-signal): validate invalid command line	yuKing123-king	+190/-29	需修改	BUILTIN 测试入口未接入 test/Makefile，不可达；register_signal 残留
#102	fix(trace-exec): reject invalid command line arg	yuKing123-king	+135/-22	需修改	BUILTIN 入口同样未接入构建；-h 退出码 0→1 属未说明的行为变更
#97	fix(so): reuse pinned dkapture bpf objects corre	yuKing123-king	+1/-1	需修改	test mock 仍按 map- 前缀命名 pin，合并后 gtest FindMap 用例失败
#35	fix(pagefault): add max_entries and value_size f	yuKing123-king	+7/-3	可合并	修复真实，但已被 main 上等效修复 1e90486 取代，建议确认后关闭
#32	fix(peek-fd): correct args field name from mvlen	yuKing123-king	+1/-1	可合并	无功能风险；标题/描述与实际改动方向不符
#30	fix(trace-signal): avoid inflight event key coll	yuKing123-king	+36/-22	需修改	sys_exit_kill 的 !rule 早退路径仍泄漏 inflight 条目
#29	fix(syscall-stat): replace exec fexit with kprob	yuKing123-king	+93/-13	阻塞	exec 路径从 struct filename* 本身读字符串，-f 过滤将完全失效

---

本 PR 评审详情

作者: JoeSergen | 规模: +28/-7 | 文件: 4
结论: 需修改
主要风险: 捆绑了与 open 状态的 PR #112 完全相同的 fs_watch 一行修复，跨 PR 重复且违反单一职责；核心 RAII 修复本身正确且必要。

总体结论: 用显式清理替换构造函数 catch 中的 this->~ClassName() 方向正确。逐一核对：所有涉及的裸指针成员均有 NSDMI（so/bpf-manager.h:20-22、so/data-map.h:34-39、so/ring-buffer.h:18-33 匿名联合内 shm_ctl/spinlock/mirror_shm = nullptr），catch 中对尚未构造的成员执行 SAFE_DELETE 安全。RingBuffer Normal 构造把 type=RING_BUF_TYPE_NORMAL 提前到 try 之前（so/ring-buffer.cpp:136），修复了真实崩溃路径：旧代码首个 try 抛异常时析构按默认 type==RING_BUF_TYPE_BPF 走 BPF 分支，对刚分配的 spinlock/shm_ctl 不做 delete。BPFManager 旧代码在 SpinLock 构造抛出时以 bpf_ref_cnt==nullptr 进入析构并执行 --(*bpf_ref_cnt)（so/bpf-manager.cpp:226-250），是必然的空指针解引用，新写法消除了该 UB。

主要问题:

• (P2) so/dkapture.cpp:388 — commit aaa0f19 的 fs_watch 修复（trace_file_init→mountsnoop_init）与仍然 open 的 PR fix: fs_watch calls trace_file_init instead of mountsnoop_init with path argument #112 的 diff 逐字节相同（两者均为 so/dkapture.cpp 同一位置同一替换），两个 PR 先后合并必然产生冲突/重复，且该修复与 PR 标题"构造函数手动析构"主题完全无关。建议从本 PR 剔除该 commit（rebase 丢弃），保留 fix: fs_watch calls trace_file_init instead of mountsnoop_init with path argument #112 单独跟踪。

次要建议:

• so/data-map.cpp:60-64 — catch 中清理顺序（m_bpf_rb→m_lock→m_rb→m_bpf→m_shm）与析构函数（so/data-map.cpp:66-74，m_bpf_rb→m_bpf→m_lock→m_rb→m_shm）不一致；当前各成员析构互不依赖故无实际危害，但建议与析构顺序保持一致，降低日后新增成员依赖时的出错概率。
• so/bpf-manager.cpp:237-247 — err_out 路径在全量初始化完成后 throw，无任何清理：构造未完成故析构不会运行，m_obj/已 pin 的 map/共享内存全部泄漏。此为预先存在的问题（本 diff 未触及），不阻塞；但既然本 PR 正在重构构造失败的清理语义，值得顺带评估在 err_out 中补清理或改为委托给私有 cleanup 方法。

亮点:

• ring-buffer.cpp:116-130 BPF 构造 err_out 的显式 munmap/close 与析构 BPF 分支语义等价且更严谨：显式排除 MAP_FAILED（析构用 (ulong)ptr > 0 判断），epoll_fd >= 0 比析构中的 > 0 更正确（fd 0 合法）。
• Normal 构造两个 catch 的 SAFE_DELETE 集合与实际失败点精确对应：mirror_shm 的 new 抛出时该成员仍为 NSDMI nullptr，无需也未被清理。

commit message: 两个 commit 均有 Signed-off-by、动机与变更列表清晰；仓库既有 fix(scope): 与 fix: 混用风格，"fix:" 无作用域可接受；但 commit 1（fs_watch）与本 PR 主题无关，见主要问题。

已有讨论: 无（issue 评论与行级评论均为空）。