pileup/mpileup: lazy-copy Bam::Record on first record() call

Author: kojix2

lib/hts/bam/pileup.rb

@@ -18,24 +18,23 @@ class PileupRecord
         def initialize(entry, header)
           @entry  = entry
           @header = header
+          @record = nil
         end
 
-        # Lightweight read-only view over the underlying bam1_t without taking ownership.
+        # Return Bam::Record. On the first call, duplicate the underlying bam1_t (bam_dup1)
+        # so the record becomes safe to keep beyond the current pileup step. Subsequent calls
+        # return the cached Bam::Record instance.
+        # NOTE: Without duplication, bam1_t memory may be reused by HTSlib on the next step.
         def record
-          @record_view ||= ReadView.new(@entry[:b])
-        end
+          return @record if @record
 
-        class ReadView
-          def initialize(bam1_ptr)
-            # Accept either FFI::Pointer or FFI::Struct (Managed/Unmanaged); store as raw pointer
-            @bam1_ptr = bam1_ptr.is_a?(FFI::Pointer) ? bam1_ptr : bam1_ptr.to_ptr
-          end
+          # Normalize to a raw pointer and duplicate to obtain owned memory.
+          b_ptr = @entry[:b].is_a?(FFI::Pointer) ? @entry[:b] : @entry[:b].to_ptr
+          dup_ptr = HTS::LibHTS.bam_dup1(b_ptr)
+          raise "bam_dup1 failed" if dup_ptr.null?
 
-          def base(i)
-            view = HTS::LibHTS::Bam1View.new(@bam1_ptr)
-            seq_ptr = HTS::LibHTS.bam_get_seq(view)
-            HTS::Bam::Record::SEQ_NT16_STR[HTS::LibHTS.bam_seqi(seq_ptr, i)]
-          end
+          # Build a Bam::Record backed by the duplicated bam1_t.
+          @record = HTS::Bam::Record.new(@header, dup_ptr)
         end
 
         def query_position

test/bam/mpileup_test.rb

@@ -43,4 +43,26 @@ def test_multipileup_accepts_bam_instances
       b2.close
     end
   end
+
+  def test_multipileup_record_lazy_copy
+    inputs = [Fixtures["moo.bam"], Fixtures["moo.bam"]]
+    mp = HTS::Bam::Mpileup.new(inputs)
+    begin
+      kept = nil
+      mp.each do |columns|
+        # find first column with at least one alignment in any input
+        col = columns.find { |c| !c.alignments.empty? }
+        next unless col
+
+        kept = col.alignments.first.record
+        break
+      end
+      refute_nil kept
+      # Should be usable after iteration moved on
+      assert_kind_of Integer, kept.len
+      assert_includes %w[= A C G T M R S V W Y H K D B N], kept.base(0)
+    ensure
+      mp.close
+    end
+  end
 end

test/bam/pileup_test.rb

@@ -39,4 +39,22 @@ def test_pileup_basic_each
       assert_operator count, :>, 0
     end
   end
+
+  def test_pileup_record_persists_beyond_step
+    HTS::Bam.open(Fixtures["moo.bam"]) do |bam|
+      kept = nil
+      # take a record from the first non-empty column and keep it
+      bam.pileup do |col|
+        next if col.alignments.empty?
+
+        kept = col.alignments.first.record
+        break
+      end
+      refute_nil kept
+      # Access after the pileup iterator has moved on; should be safe
+      assert_kind_of String, kept.qname
+      # Access a base in the read sequence
+      assert_includes %w[= A C G T M R S V W Y H K D B N], kept.base(0)
+    end
+  end
 end