Fix AxiStreamDma stop races and zero-copy lifetime

[bengineerd]

Description

Fix AxiStreamDma shutdown so stop() is safe while stream and driver operations are still in flight. The previous code could close fd_ while another thread was inside a driver call such as zero-copy allocation, transmit, buffer return, or a getter. This is a use-after-close race on an OS file descriptor and can also allow the fd number to be reused for an unrelated resource.

This also keeps the shared zero-copy DMA mapping alive until ~AxiStreamDma(), so retained Rogue frames do not point at unmapped DMA memory after a user calls stop().

Details

• Add stopped_ state so new stream work is rejected once shutdown begins, while post-stop getters return safe default values.
• Add an fd_ guard backed by std::shared_timed_mutex: driver operations take a shared lock, and stop() takes the exclusive lock before closing the descriptor. This guarantees close(fd_) cannot run while a guarded driver call is using that fd.
• Apply the guard to acceptReq(), acceptFrame(), retBuffer(), driver getters, debug setup, and DMA ack.
• Move shared descriptor cleanup from stop() to the destructor so zero-copy buffers retained after stop() remain backed by valid mapped memory.
• Keep stop() idempotent and join the RX thread based on joinable().
• Add Linux-only C++ regression tests with an LD_PRELOAD fake aes-stream-driver shim that blocks selected driver calls to deterministically prove stop() waits instead of closing during active calls.

Validation:

• Linux NO_PYTHON C++ hardware tests pass for retained zero-copy lifetime and stop-vs-driver-call races.
• Reverting the production AxiStreamDma changes while keeping the new tests fails as expected: mappings are released at stop(), blocked calls can overlap descriptor close, and the fake shim detects close-during-driver-call.
• macOS local check: git diff --check, cmake --build build --target rogue-core-shared. The new hardware tests are Linux-only and are not registered on macOS.

JIRA

N/A

Related

Related to #1276 teardown review.

[Copilot]

Pull request overview

This PR adjusts rogue::hardware::axi::AxiStreamDma teardown semantics for zero-copy so that stop() closes only the per-instance DMA file descriptor while keeping the shared mapped DMA buffers alive until ~AxiStreamDma(), preventing downstream zero-copy frames retained past stop() from referencing unmapped memory.

Changes:

• Move shared descriptor (desc_) cleanup from stop() into ~AxiStreamDma().
• Add fdMtx_ and use it to serialize deferred dmaRetIndex() calls against fd_ closure.
• Remove unused locals in retBuffer() and update stop()/API comments to reflect the new lifetime model.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/rogue/hardware/axi/AxiStreamDma.cpp	Moves shared mapping cleanup to destructor; adds mutex-guarded fd close and mutex-guarded zero-copy buffer return.
include/rogue/hardware/axi/AxiStreamDma.h	Adds <mutex>, introduces fdMtx_, and updates stop() documentation to reflect new lifetime behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

[slacrherbst]

I have a few concerns with this. I guess which is the error this is really solving. Since the fd_ value is only written twice and open and close and all other blocks read, I would suspect we would be fine without a lock. Were there actual cases where we were seeing segfaults on close because accept_frame was called? Also, I can understand the issue with the allocated buffer returns; that seems like a more real problem. Anyway, we should fully understand what is being solved. My main problem is the solution creates a huge amount of object creation and destruction to implement the lock. I would have preferred a more direct, but maybe less "layered and python" like approach that limited the memory allocations at high rate. The target high rate is also why I am apprehensive about even adding locks so I want to make sure this is really necessary. If we do have to add locks around the fd access, we need to use a clean mutex without the fancy class that wraps it.

[bengineerd]

I have reworked the change locally to focus on two cases:

• Keep the shared zero-copy DMA mapping alive until AxiStreamDma destruction so retained frames do not point at unmapped memory after stop().
• Serialize deferred zero-copy dmaRetIndex() buffer returns against closing the per-instance file descriptor.

The FdGuard, stopped_ state, shared_timed_mutex, and locks around allocation, transmit, getters, debug setup, and DMA ack have been removed. The remaining synchronization is a direct std::mutex used only by retBuffer() and the descriptor-close block in stop().

The public documentation now requires callers to quiesce stream operations and driver controls/accessors before calling stop(). Deferred returns from zero-copy buffers already issued by the interface are the one operation explicitly allowed to overlap shutdown.

The regression coverage has also been narrowed to the two behaviors above: retained mapping lifetime and dmaRetIndex() versus descriptor close. The local core build and source checks pass; the runtime fake-driver regressions are Linux-only and will run in CI after the revised code is pushed.