musikr: improve iostream lifecycle mgmt

musikr/src/main/jni/shim/file_shim.cpp

@@ -2,6 +2,9 @@
 
 namespace taglib_shim
 {
+    std::unique_ptr<TagLib::FileRef> new_FileRef(std::unique_ptr<TagLib::IOStream> stream) {
+        return std::make_unique<TagLib::FileRef>(stream.release());
+    }
 
     TagLib::Ogg::Vorbis::File *File_asVorbis(TagLib::File *file)
     {

musikr/src/main/jni/shim/file_shim.hpp

@@ -16,6 +16,7 @@
 
 namespace taglib_shim
 {
+    std::unique_ptr<TagLib::FileRef> new_FileRef(TagLib::IOStream *stream);
 
     TagLib::Ogg::Vorbis::File *File_asVorbis(TagLib::File *file);
     TagLib::Ogg::Opus::File *File_asOpus(TagLib::File *file);

musikr/src/main/jni/shim/iostream_shim.cpp

@@ -4,57 +4,57 @@
 #include <vector>
 #include "metadatajni/src/taglib/bridge.rs.h"
 
-// These are the functions we'll define in Rust
-extern "C"
-{
-    const char *rust_stream_name(const void *stream);
-    size_t rust_stream_read(void *stream, uint8_t *buffer, size_t length);
-    void rust_stream_write(void *stream, const uint8_t *data, size_t length);
-    void rust_stream_seek(void *stream, int64_t offset, int32_t whence);
-    void rust_stream_truncate(void *stream, int64_t length);
-    int64_t rust_stream_tell(const void *stream);
-    int64_t rust_stream_length(const void *stream);
-    bool rust_stream_is_readonly(const void *stream);
-}
-
 namespace taglib_shim
 {
 
-    // Factory function to create a new RustIOStream
-    std::unique_ptr<RustIOStream> new_RustIOStream(BridgeStream& stream)
+    // C++ implementation of TagLib::IOStream that delegates to Rust
+    class WrappedRsIOStream : public TagLib::IOStream
     {
-        return std::unique_ptr<RustIOStream>(new RustIOStream(stream));
-    }
+    public:
+        explicit WrappedRsIOStream(RsIOStream& stream);
+        ~WrappedRsIOStream() override;
 
-    // Factory function to create a FileRef from a stream
-    std::unique_ptr<TagLib::FileRef> new_FileRef_from_stream(std::unique_ptr<RustIOStream> stream)
-    {
-        return std::make_unique<TagLib::FileRef>(stream.release(), true);
-    }
-    
-    RustIOStream::RustIOStream(BridgeStream& stream) : rust_stream(stream) {}
+        // TagLib::IOStream interface implementation
+        TagLib::FileName name() const override;
+        TagLib::ByteVector readBlock(size_t length) override;
+        void writeBlock(const TagLib::ByteVector &data) override;
+        void insert(const TagLib::ByteVector &data, TagLib::offset_t start = 0, size_t replace = 0) override;
+        void removeBlock(TagLib::offset_t start = 0, size_t length = 0) override;
+        void seek(TagLib::offset_t offset, Position p = Beginning) override;
+        void clear() override;
+        void truncate(TagLib::offset_t length) override;
+        TagLib::offset_t tell() const override;
+        TagLib::offset_t length() override;
+        bool readOnly() const override;
+        bool isOpen() const override;
 
-    RustIOStream::~RustIOStream() = default;
+    private:
+        RsIOStream& rust_stream;
+    };
 
-    TagLib::FileName RustIOStream::name() const
+    WrappedRsIOStream::WrappedRsIOStream(RsIOStream& stream) : rust_stream(stream) {}
+
+    WrappedRsIOStream::~WrappedRsIOStream() = default;
+
+    TagLib::FileName WrappedRsIOStream::name() const
     {
         return rust::string(rust_stream.name()).c_str();
     }
 
-    TagLib::ByteVector RustIOStream::readBlock(size_t length)
+    TagLib::ByteVector WrappedRsIOStream::readBlock(size_t length)
     {
         std::vector<uint8_t> buffer(length);
         size_t bytes_read = rust_stream.read(rust::Slice<uint8_t>(buffer.data(), length));
         return TagLib::ByteVector(reinterpret_cast<char *>(buffer.data()), bytes_read);
     }
 
-    void RustIOStream::writeBlock(const TagLib::ByteVector &data)
+    void WrappedRsIOStream::writeBlock(const TagLib::ByteVector &data)
     {
         rust_stream.write(rust::Slice<const uint8_t>(
             reinterpret_cast<const uint8_t *>(data.data()), data.size()));
     }
 
-    void RustIOStream::insert(const TagLib::ByteVector &data, TagLib::offset_t start, size_t replace)
+    void WrappedRsIOStream::insert(const TagLib::ByteVector &data, TagLib::offset_t start, size_t replace)
     {
         // Save current position
         auto current = tell();
@@ -75,7 +75,7 @@ namespace taglib_shim
         seek(current);
     }
 
-    void RustIOStream::removeBlock(TagLib::offset_t start, size_t length)
+    void WrappedRsIOStream::removeBlock(TagLib::offset_t start, size_t length)
     {
         if (length == 0)
             return;
@@ -101,7 +101,7 @@ namespace taglib_shim
         seek(current);
     }
 
-    void RustIOStream::seek(TagLib::offset_t offset, Position p)
+    void WrappedRsIOStream::seek(TagLib::offset_t offset, Position p)
     {
         int32_t whence;
         switch (p)
@@ -121,35 +121,41 @@ namespace taglib_shim
         rust_stream.seek(offset, whence);
     }
 
-    void RustIOStream::clear()
+    void WrappedRsIOStream::clear()
     {
         truncate(0);
         seek(0);
     }
 
-    void RustIOStream::truncate(TagLib::offset_t length)
+    void WrappedRsIOStream::truncate(TagLib::offset_t length)
     {
         rust_stream.truncate(length);
     }
 
-    TagLib::offset_t RustIOStream::tell() const
+    TagLib::offset_t WrappedRsIOStream::tell() const
     {
         return rust_stream.tell();
     }
 
-    TagLib::offset_t RustIOStream::length()
+    TagLib::offset_t WrappedRsIOStream::length()
     {
         return rust_stream.length();
     }
 
-    bool RustIOStream::readOnly() const
+    bool WrappedRsIOStream::readOnly() const
     {
         return rust_stream.is_readonly();
     }
 
-    bool RustIOStream::isOpen() const
+    bool WrappedRsIOStream::isOpen() const
     {
         return true; // If we have a stream, it's open
     }
 
+    // Factory function to create a new RustIOStream
+    std::unique_ptr<TagLib::IOStream> wrap_RsIOStream(RsIOStream& stream)
+    {
+        return std::unique_ptr<TagLib::IOStream>(new WrappedRsIOStream(stream));
+    }
+
 } // namespace taglib_shim

musikr/src/main/jni/shim/iostream_shim.hpp

@@ -7,39 +7,10 @@
 #include "rust/cxx.h"
 
 // Forward declare the bridge type
-struct BridgeStream;
+struct RsIOStream;
 
 namespace taglib_shim
 {
-
-    // C++ implementation of TagLib::IOStream that delegates to Rust
-    class RustIOStream : public TagLib::IOStream
-    {
-    public:
-        explicit RustIOStream(BridgeStream& stream);
-        ~RustIOStream() override;
-
-        // TagLib::IOStream interface implementation
-        TagLib::FileName name() const override;
-        TagLib::ByteVector readBlock(size_t length) override;
-        void writeBlock(const TagLib::ByteVector &data) override;
-        void insert(const TagLib::ByteVector &data, TagLib::offset_t start = 0, size_t replace = 0) override;
-        void removeBlock(TagLib::offset_t start = 0, size_t length = 0) override;
-        void seek(TagLib::offset_t offset, Position p = Beginning) override;
-        void clear() override;
-        void truncate(TagLib::offset_t length) override;
-        TagLib::offset_t tell() const override;
-        TagLib::offset_t length() override;
-        bool readOnly() const override;
-        bool isOpen() const override;
-
-    private:
-        BridgeStream& rust_stream;
-    };
-
     // Factory functions with external linkage
-    std::unique_ptr<RustIOStream> new_RustIOStream(BridgeStream& stream);
-    
-    std::unique_ptr<TagLib::FileRef> new_FileRef_from_stream(std::unique_ptr<RustIOStream> stream);
-
+    std::unique_ptr<TagLib::IOStream> wrap_RsIOStream(RsIOStream& stream);
 } // namespace taglib_shim

musikr/src/main/jni/src/jstream.rs

@@ -1,4 +1,4 @@
-use crate::taglib::file::IOStream;
+use crate::taglib::iostream::IOStream;
 use jni::objects::{JObject, JValue};
 use std::io::{Read, Seek, SeekFrom, Write};
 use crate::SharedEnv;
@@ -18,7 +18,7 @@ impl<'local, 'a> JInputStream<'local> {
 }
 
 impl<'local> IOStream for JInputStream<'local> {
-    fn name(&mut self) -> String {
+    fn name(&self) -> String {
         // Call the Java name() method safely
         let name = self
             .env

musikr/src/main/jni/src/taglib/bridge.rs

@@ -1,18 +1,20 @@
+use super::iostream::DynIOStream;
+
 #[cxx::bridge]
 mod bridge_impl {
     // Expose Rust IOStream to C++
     extern "Rust" {
-        #[cxx_name = "BridgeStream"]
-        type TIOStream<'a>;
+        #[cxx_name = "RsIOStream"]
+        type DynIOStream<'a>;
 
-        fn name(self: &mut TIOStream<'_>) -> String;
-        fn read(self: &mut TIOStream<'_>, buffer: &mut [u8]) -> usize;
-        fn write(self: &mut TIOStream<'_>, data: &[u8]);
-        fn seek(self: &mut TIOStream<'_>, offset: i64, whence: i32);
-        fn truncate(self: &mut TIOStream<'_>, length: i64);
-        fn tell(self: &mut TIOStream<'_>) -> i64;
-        fn length(self: &mut TIOStream<'_>) -> i64;
-        fn is_readonly(self: &TIOStream<'_>) -> bool;
+        fn name(self: &mut DynIOStream<'_>) -> String;
+        fn read(self: &mut DynIOStream<'_>, buffer: &mut [u8]) -> usize;
+        fn write(self: &mut DynIOStream<'_>, data: &[u8]);
+        fn seek(self: &mut DynIOStream<'_>, offset: i64, whence: i32);
+        fn truncate(self: &mut DynIOStream<'_>, length: i64);
+        fn tell(self: &mut DynIOStream<'_>) -> i64;
+        fn length(self: &mut DynIOStream<'_>) -> i64;
+        fn is_readonly(self: &mut DynIOStream<'_>) -> bool;
     }
 
     #[namespace = "taglib_shim"]
@@ -22,12 +24,14 @@ mod bridge_impl {
         include!("taglib/tstringlist.h");
         include!("taglib/vorbisfile.h");
         include!("taglib/xiphcomment.h");
+        include!("taglib/tiostream.h");
         include!("shim/iostream_shim.hpp");
         include!("shim/file_shim.hpp");
         include!("shim/tk_shim.hpp");
 
-        #[cxx_name = "RustIOStream"]
-        type RustIOStream;
+        #[namespace = "TagLib"]
+        #[cxx_name = "IOStream"]
+        type CPPIOStream;
 
         #[namespace = "TagLib"]
         #[cxx_name = "FileRef"]
@@ -38,9 +42,9 @@ mod bridge_impl {
         fn thisFile(self: Pin<&TFileRef>) -> *mut BaseFile;
 
         // Create a RustIOStream from a BridgeStream
-        unsafe fn new_RustIOStream(stream: Pin<&mut TIOStream>) -> UniquePtr<RustIOStream>;
+        unsafe fn wrap_RsIOStream(stream: Pin<&mut DynIOStream>) -> UniquePtr<CPPIOStream>;
         // Create a FileRef from an iostream
-        fn new_FileRef_from_stream(stream: UniquePtr<RustIOStream>) -> UniquePtr<TFileRef>;
+        unsafe fn new_FileRef(stream: *mut CPPIOStream) -> UniquePtr<TFileRef>;
 
         #[namespace = "TagLib"]
         #[cxx_name = "File"]
@@ -152,60 +156,3 @@ mod bridge_impl {
 }
 
 pub use bridge_impl::*;
-
-use std::io::SeekFrom;
-use std::pin::Pin;
-use super::file::IOStream;
-
-#[repr(C)]
-pub(super) struct TIOStream<'a>(Box<dyn IOStream + 'a>);
-
-impl<'a> TIOStream<'a> {
-    pub fn new<T: IOStream + 'a>(stream: T) -> Pin<Box<Self>> {
-        Box::pin(TIOStream(Box::new(stream)))
-    }
-
-
-    // Implement the exposed functions for cxx bridge
-    pub fn name(&mut self) -> String {
-        self.0.name()
-    }
-
-    pub fn read(&mut self, buffer: &mut [u8]) -> usize {
-        self.0.read(buffer).unwrap_or(0)
-    }
-
-    pub fn write(&mut self, data: &[u8]) {
-        self.0.write_all(data).unwrap();
-    }
-
-    pub fn seek(&mut self, offset: i64, whence: i32) {
-        let pos = match whence {
-            0 => SeekFrom::Start(offset as u64),
-            1 => SeekFrom::Current(offset),
-            2 => SeekFrom::End(offset),
-            _ => panic!("Invalid seek whence"),
-        };
-        self.0.seek(pos).unwrap();
-    }
-
-    pub fn truncate(&mut self, length: i64) {
-        self.0.seek(SeekFrom::Start(length as u64)).unwrap();
-        // TODO: Actually implement truncate once we have a better trait bound
-    }
-
-    pub fn tell(&mut self) -> i64 {
-        self.0.seek(SeekFrom::Current(0)).unwrap() as i64
-    }
-
-    pub fn length(&mut self) -> i64 {
-        let current = self.0.seek(SeekFrom::Current(0)).unwrap();
-        let end = self.0.seek(SeekFrom::End(0)).unwrap();
-        self.0.seek(SeekFrom::Start(current)).unwrap();
-        end as i64
-    }
-
-    pub fn is_readonly(&self) -> bool {
-        self.0.is_readonly()
-    }
-}

musikr/src/main/jni/src/taglib/file.rs

@@ -1,22 +1,22 @@
 use cxx::UniquePtr;
-use super::bridge::{self, TFileRef, TIOStream};
+use super::bridge::{self, TFileRef};
+use super::iostream::{IOStream, BridgedIOStream};
 pub use super::bridge::{BaseFile as File, AudioProperties};
 use super::xiph::{OpusFile, VorbisFile, FLACFile};
-use std::io::{Read, Write, Seek};
 use std::pin::Pin;
 
 pub struct FileRef<'a> {
-    stream: Pin<Box<TIOStream<'a>>>,
+    stream: BridgedIOStream<'a>,
     file_ref: UniquePtr<TFileRef>
 }
 
 impl <'a> FileRef<'a> {
     pub fn new<T : IOStream + 'a>(stream: T) -> FileRef<'a> {
-        let mut bridge_stream  = TIOStream::new(stream);
-        let iostream = unsafe { bridge::new_RustIOStream(bridge_stream.as_mut()) };
-        let file_ref = bridge::new_FileRef_from_stream(iostream);
+        let stream  = BridgedIOStream::new(stream);
+        let cpp_stream = stream.cpp_stream().as_mut_ptr();
+        let file_ref = unsafe { bridge::new_FileRef(cpp_stream) };
         FileRef {
-            stream: bridge_stream,
+            stream,
             file_ref
         }
     }
@@ -195,8 +195,3 @@ impl <'a> Drop for FileRef<'a> {
         }
     }
 }
-
-pub trait IOStream: Read + Write + Seek {
-    fn name(&mut self) -> String;
-    fn is_readonly(&self) -> bool;
-}

musikr/src/main/jni/src/taglib/iostream.rs

@@ -0,0 +1,93 @@
+
+use super::bridge::{self, CPPIOStream};
+use std::io::{Read, Write, Seek, SeekFrom};
+use std::pin::Pin;
+use cxx::UniquePtr;
+
+pub trait IOStream : Read + Write + Seek {
+    fn name(&self) -> String;
+    fn is_readonly(&self) -> bool;
+}
+
+
+pub(super) struct BridgedIOStream<'a> {
+    rs_stream: Pin<Box<DynIOStream<'a>>>,
+    cpp_stream: UniquePtr<CPPIOStream>
+}
+
+impl<'a> BridgedIOStream<'a> {
+    pub fn new<T : IOStream + 'a>(stream: T) -> Self {
+        let mut rs_stream = Box::pin(DynIOStream(Box::new(stream)));
+        let cpp_stream = unsafe { bridge::wrap_RsIOStream(rs_stream.as_mut()) };
+        BridgedIOStream {
+            rs_stream,
+            cpp_stream
+        }
+    }
+
+    pub fn cpp_stream(&self) -> &UniquePtr<CPPIOStream> {
+        &self.cpp_stream
+    }
+}
+
+impl<'a> Drop for BridgedIOStream<'a> {
+    fn drop(&mut self) {
+        unsafe { 
+            // CPP stream references the rust stream, so it must be dropped first
+            std::ptr::drop_in_place(&mut self.cpp_stream);
+            std::ptr::drop_in_place(&mut self.rs_stream);
+        };
+    }
+}
+
+#[repr(C)]
+pub(super) struct DynIOStream<'a>(Box<dyn IOStream + 'a>);
+
+impl<'a> DynIOStream<'a> {
+    pub fn new<T: IOStream + 'a>(stream: T) -> Self {
+        DynIOStream(Box::new(stream))
+    }
+
+    // Implement the exposed functions for cxx bridge
+    pub fn name(&mut self) -> String {
+        self.0.name()
+    }
+
+    pub fn read(&mut self, buffer: &mut [u8]) -> usize {
+        self.0.read(buffer).unwrap_or(0)
+    }
+
+    pub fn write(&mut self, data: &[u8]) {
+        self.0.write_all(data).unwrap();
+    }
+
+    pub fn seek(&mut self, offset: i64, whence: i32) {
+        let pos = match whence {
+            0 => SeekFrom::Start(offset as u64),
+            1 => SeekFrom::Current(offset),
+            2 => SeekFrom::End(offset),
+            _ => panic!("Invalid seek whence"),
+        };
+        self.0.seek(pos).unwrap();
+    }
+
+    pub fn truncate(&mut self, length: i64) {
+        self.0.seek(SeekFrom::Start(length as u64)).unwrap();
+        // TODO: Actually implement truncate once we have a better trait bound
+    }
+
+    pub fn tell(&mut self) -> i64 {
+        self.0.seek(SeekFrom::Current(0)).unwrap() as i64
+    }
+
+    pub fn length(&mut self) -> i64 {
+        let current = self.0.seek(SeekFrom::Current(0)).unwrap();
+        let end = self.0.seek(SeekFrom::End(0)).unwrap();
+        self.0.seek(SeekFrom::Start(current)).unwrap();
+        end as i64
+    }
+
+    pub fn is_readonly(&self) -> bool {
+        self.0.is_readonly()
+    }
+}

musikr/src/main/jni/src/taglib/mod.rs

@@ -3,3 +3,4 @@ mod bridge;
 pub mod file;
 pub mod tk;
 pub mod xiph;
+pub mod iostream;