Fix AxiStreamDma stop races and zero-copy lifetime

include/rogue/hardware/axi/AxiStreamDma.h

@@ -23,6 +23,7 @@
 #include <atomic>
 #include <map>
 #include <memory>
+#include <mutex>
 #include <string>
 #include <thread>
 
@@ -104,6 +105,9 @@ class AxiStreamDma : public rogue::interfaces::stream::Master, public rogue::int
     // Process-local descriptor for TX/RX operations and dest mask programming.
     int32_t fd_;
 
+    // Serializes fd_ close against deferred zero-copy buffer returns.
+    std::mutex fdMtx_;
+
     // Destination selector used when transmitting frames.
     uint32_t dest_;
 
@@ -204,7 +208,14 @@ class AxiStreamDma : public rogue::interfaces::stream::Master, public rogue::int
     /** @brief Destroys the interface and stops background activity. */
     ~AxiStreamDma();
 
-    /** @brief Stops RX thread and closes DMA file descriptors. */
+    /**
+     * @brief Stops RX thread and closes the per-instance DMA file descriptor.
+     *
+     * @details
+     * The shared zero-copy DMA mapping remains valid until destruction so
+     * downstream Rogue buffers retained after `stop()` do not reference
+     * unmapped memory.
+     */
     void stop();
 
     /**

src/rogue/hardware/axi/AxiStreamDma.cpp

@@ -294,6 +294,11 @@ rha::AxiStreamDma::AxiStreamDma(std::string path, uint32_t dest, bool ssiEnable)
 //! Close the device
 rha::AxiStreamDma::~AxiStreamDma() {
     this->stop();
+
+    if (desc_) {
+        closeShared(desc_);
+        desc_.reset();
+    }
 }
 
 void rha::AxiStreamDma::stop() {
@@ -314,19 +319,15 @@ void rha::AxiStreamDma::stop() {
     }
     thread_.reset();
 
-    // Release the shared descriptor exactly once even if stop() is called
-    // again (or runThread() self-exited and the user called stop() before
-    // ~AxiStreamDma()): clearing desc_ after closeShared() prevents a
-    // double-decrement of openCount.
-    if (desc_) {
-        closeShared(desc_);
-        desc_.reset();
-    }
-
-    // Close the per-instance fd exactly once.
-    if (fd_ >= 0) {
-        ::close(fd_);
-        fd_ = -1;
+    // Close the per-instance fd exactly once.  The shared DMA mapping remains
+    // open until ~AxiStreamDma() because zero-copy Rogue buffers can outlive a
+    // user-initiated stop() and still point into desc_->rawBuff.
+    {
+        std::lock_guard<std::mutex> lock(fdMtx_);
+        if (fd_ >= 0) {
+            ::close(fd_);
+            fd_ = -1;
+        }
     }
 }
 
@@ -358,10 +359,8 @@ ris::FramePtr rha::AxiStreamDma::acceptReq(uint32_t size, bool zeroCopyEn) {
     ris::FramePtr frame;
     uint32_t buffSize;
 
-    // Reject use after stop()/teardown.  stop() resets the shared-descriptor
-    // shared_ptr to nullptr and closes the per-instance fd, so a post-stop
-    // call would otherwise segfault on the buffer-size read below instead
-    // of throwing a clean GeneralError.
+    // Reject use after stop()/teardown.  stop() closes the per-instance fd,
+    // and destruction releases the shared descriptor.
     if (!desc_ || fd_ < 0)
         throw rogue::GeneralError("AxiStreamDma::acceptReq",
                                   "instance has been stopped or did not finish construction");
@@ -442,10 +441,10 @@ void rha::AxiStreamDma::acceptFrame(ris::FramePtr frame) {
     uint32_t cont;
     bool emptyFrame;
 
-    // Reject use after stop()/teardown.  stop() releases the shared descriptor
-    // and closes fd_, so the inner-loop fd_ guard would still catch this, but
-    // that fires only after frame->lock() and buffer iteration have already
-    // run.  Fail-fast at entry for a cleaner error path, mirroring acceptReq().
+    // Reject use after stop()/teardown.  The inner-loop fd_ guard would still
+    // catch this, but that fires only after frame->lock() and buffer iteration
+    // have already run.  Fail-fast at entry for a cleaner error path, mirroring
+    // acceptReq().
     if (!desc_ || fd_ < 0)
         throw rogue::GeneralError("AxiStreamDma::acceptFrame",
                                   "instance has been stopped or did not finish construction");
@@ -557,16 +556,14 @@ void rha::AxiStreamDma::acceptFrame(ris::FramePtr frame) {
 //! Return a buffer
 void rha::AxiStreamDma::retBuffer(uint8_t* data, uint32_t meta, uint32_t size) {
     rogue::GilRelease noGil;
-    uint32_t ret[100];
-    uint32_t count;
-    uint32_t x;
 
     // Buffer is zero copy as indicated by bit 31
     if ((meta & 0x80000000) != 0) {
         // Device is open and buffer is not stale
         // Bit 30 indicates buffer has already been returned to hardware
-        if ((fd_ >= 0) && ((meta & 0x40000000) == 0)) {
-            if (dmaRetIndex(fd_, meta & 0x3FFFFFFF) < 0)
+        if ((meta & 0x40000000) == 0) {
+            std::lock_guard<std::mutex> lock(fdMtx_);
+            if (fd_ >= 0 && dmaRetIndex(fd_, meta & 0x3FFFFFFF) < 0)
                 throw(rogue::GeneralError("AxiStreamDma::retBuffer", "AXIS Return Buffer Call Failed!!!!"));
         }
         decCounter(size);