musikr: improve lifecycles

musikr/src/main/jni/src/lib.rs

@@ -8,6 +8,7 @@ mod taglib;
 mod jstream;
 
 use taglib::file_ref::FileRef;
+use taglib::file::File;
 use jstream::JInputStream;
 
 type SharedEnv<'local> = Rc<RefCell<JNIEnv<'local>>>;

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

@@ -1,12 +1,12 @@
 use super::bridge::CppAudioProperties;
 use std::pin::Pin;
 
-pub struct AudioProperties<'a> {
+pub struct AudioProperties<'file_ref> {
-    this: Pin<&'a CppAudioProperties>
+    this: Pin<&'file_ref CppAudioProperties>
 }
 
-impl<'a> AudioProperties<'a> {
+impl<'file_ref> AudioProperties<'file_ref> {
-    pub(super) fn new(this: Pin<&'a CppAudioProperties>) -> Self {
+    pub(super) fn new(this: Pin<&'file_ref CppAudioProperties>) -> Self {
         Self { this }
     }
 

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

@@ -5,17 +5,17 @@ use super::ogg::OpusFile;
 use super::ogg::VorbisFile;
 use super::flac::FLACFile;
 
-pub struct File<'a> {
+pub struct File<'file_ref> {
-    this: Pin<&'a mut CPPFile>
+    this: Pin<&'file_ref mut CPPFile>
 }
 
 
-impl<'a> File<'a> {
+impl<'file_ref> File<'file_ref> {
-    pub(super) fn new(this: Pin<&'a mut CPPFile>) -> Self {
+    pub(super) fn new(this: Pin<&'file_ref mut CPPFile>) -> Self {
         Self { this }
     }
 
-    pub fn audio_properties(&self) -> Option<AudioProperties<'a>> {
+    pub fn audio_properties(&self) -> Option<AudioProperties<'file_ref>> {
         let props_ptr =  self.this.as_ref().audioProperties();
         let props_ref = unsafe {
             // SAFETY:
@@ -30,7 +30,7 @@ impl<'a> File<'a> {
         props_pin.map(|props| AudioProperties::new(props))
     }
 
-    pub fn as_opus(&mut self) -> Option<OpusFile<'a>> {
+    pub fn as_opus(&mut self) -> Option<OpusFile<'file_ref>> {
         let opus_file = unsafe {
             // SAFETY:
             // This FFI function will be a simple C++ dynamic_cast, which checks if
@@ -51,7 +51,7 @@ impl<'a> File<'a> {
         opus_pin.map(|opus| OpusFile::new(opus))
     }
 
-    pub fn as_vorbis(&mut self) -> Option<VorbisFile<'a>> {
+    pub fn as_vorbis(&mut self) -> Option<VorbisFile<'file_ref>> {
         let vorbis_file = unsafe {
             // SAFETY:
             // This FFI function will be a simple C++ dynamic_cast, which checks if
@@ -72,7 +72,7 @@ impl<'a> File<'a> {
         vorbis_pin.map(|vorbis| VorbisFile::new(vorbis))
     }
 
-    pub fn as_flac(&mut self) -> Option<FLACFile<'a>> {
+    pub fn as_flac(&mut self) -> Option<FLACFile<'file_ref>> {
         let flac_file = unsafe {
             // SAFETY:
             // This FFI function will be a simple C++ dynamic_cast, which checks if
@@ -94,7 +94,7 @@ impl<'a> File<'a> {
     }
 }
 
-impl<'a> Drop for File<'a> {
+impl<'file_ref> Drop for File<'file_ref> {
     fn drop(&mut self) {
         unsafe {
             std::ptr::drop_in_place(&mut self.this);

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

@@ -4,13 +4,13 @@ use super::iostream::{BridgedIOStream, IOStream};
 use cxx::UniquePtr;
 use std::pin::Pin;
 
-pub struct FileRef<'a> {
+pub struct FileRef<'io> {
-    stream: BridgedIOStream<'a>,
+    stream: BridgedIOStream<'io>,
     this: UniquePtr<CPPFileRef>,
 }
 
-impl<'a> FileRef<'a> {
+impl<'io> FileRef<'io> {
-    pub fn new<T: IOStream + 'a>(stream: T) -> FileRef<'a> {
+    pub fn new<T: IOStream + 'io>(stream: T) -> FileRef<'io> {
         let stream = BridgedIOStream::new(stream);
         let cpp_stream = stream.cpp_stream().as_mut_ptr();
         let file_ref = unsafe { bridge::new_FileRef(cpp_stream) };
@@ -20,7 +20,7 @@ impl<'a> FileRef<'a> {
         }
     }
 
-    pub fn file(&self) -> Option<File<'a>> {
+    pub fn file<'file_ref>(&'file_ref self) -> Option<File<'file_ref>> {
         let file_ptr = unsafe {
             // SAFETY:
             // - This pin is only used in this unsafe scope.

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

@@ -7,16 +7,16 @@ use std::marker::PhantomData;
 use cxx::UniquePtr;
 use std::pin::Pin;
 
-pub struct FLACFile<'a> {
+pub struct FLACFile<'file_ref> {
-    this: Pin<&'a mut CPPFLACFile>
+    this: Pin<&'file_ref mut CPPFLACFile>
 }
 
-impl<'a> FLACFile<'a> {
+impl<'file_ref> FLACFile<'file_ref> {
-    pub(super) fn new(this: Pin<&'a mut CPPFLACFile>) -> Self {
+    pub(super) fn new(this: Pin<&'file_ref mut CPPFLACFile>) -> Self {
         Self { this }
     }
 
-    pub fn xiph_comments(&mut self) -> Option<XiphComment> {
+    pub fn xiph_comments(&mut self) -> Option<XiphComment<'file_ref>> {
         let this = self.this.as_mut();
         let tag = this.xiphComment(false);
         let tag_ref = unsafe {
@@ -28,25 +28,26 @@ impl<'a> FLACFile<'a> {
         tag_pin.map(|tag| XiphComment::new(tag))
     }
 
-    pub fn picture_list(&mut self) -> PictureList {
+    pub fn picture_list(&mut self) -> PictureList<'file_ref> {
         let pictures = FLACFile_pictureList(self.this.as_mut());
         PictureList::new(pictures)
     }
 }
 
-pub struct PictureList<'a> {
+pub struct PictureList<'file_ref> {
-    // PictureList is implicitly tied to the lifetime of the parent that owns it,
+    // PictureList is implicitly tied to the lifetime of the file_ref, despite us technically
-    // so we need to track that lifetime.
+    // """""owning"""" it.
-    _data: PhantomData<&'a CPPFLACPicture>,
+    _data: PhantomData<&'file_ref CPPFLACPicture>,
+    // Only in a UniquePtr because we can't marshal over ownership of the PictureList by itself over cxx.
     this: UniquePtr<CPPPictureList>,
 }
 
-impl<'a> PictureList<'a> {
+impl<'file_ref> PictureList<'file_ref> {
     pub(super) fn new(this: UniquePtr<CPPPictureList>) -> Self {
         Self { _data: PhantomData, this }
     }
 
-    pub fn to_vec(&self) -> Vec<Picture> {
+    pub fn to_vec(&self) -> Vec<Picture<'file_ref>> {
         let pictures = PictureList_to_vector(unsafe {
             // SAFETY: This pin is only used in this unsafe scope.
             // The pin is used as a C++ this pointer in the ffi call, which does
@@ -69,16 +70,16 @@ impl<'a> PictureList<'a> {
     }
 }
 
-pub struct Picture<'a> {
+pub struct Picture<'file_ref> {
-    this: Pin<&'a CPPFLACPicture>
+    this: Pin<&'file_ref CPPFLACPicture>
 }
 
-impl<'a> Picture<'a> {
+impl<'file_ref> Picture<'file_ref> {
-    pub(super) fn new(this: Pin<&'a CPPFLACPicture>) -> Self {
+    pub(super) fn new(this: Pin<&'file_ref CPPFLACPicture>) -> Self {
         Self { this }
     }
 
-    pub fn data(&self) -> ByteVector<'a> {
+    pub fn data(&self) -> ByteVector<'file_ref> {
         let data = Picture_data(self.this);
         ByteVector::new(data)
     }

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

@@ -10,13 +10,13 @@ pub trait IOStream : Read + Write + Seek {
 }
 
 
-pub(super) struct BridgedIOStream<'a> {
+pub(super) struct BridgedIOStream<'io_stream> {
-    rs_stream: Pin<Box<DynIOStream<'a>>>,
+    rs_stream: Pin<Box<DynIOStream<'io_stream>>>,
     cpp_stream: UniquePtr<CPPIOStream>
 }
 
-impl<'a> BridgedIOStream<'a> {
+impl<'io_stream> BridgedIOStream<'io_stream> {
-    pub fn new<T : IOStream + 'a>(stream: T) -> Self {
+    pub fn new<T : IOStream + 'io_stream>(stream: T) -> Self {
         let mut rs_stream = Box::pin(DynIOStream(Box::new(stream)));
         let cpp_stream = bridge::wrap_RsIOStream(rs_stream.as_mut());
         BridgedIOStream {
@@ -30,7 +30,7 @@ impl<'a> BridgedIOStream<'a> {
     }
 }
 
-impl<'a> Drop for BridgedIOStream<'a> {
+impl<'io_stream> Drop for BridgedIOStream<'io_stream> {
     fn drop(&mut self) {
         unsafe { 
             // CPP stream references the rust stream, so it must be dropped first
@@ -41,9 +41,9 @@ impl<'a> Drop for BridgedIOStream<'a> {
 }
 
 #[repr(C)]
-pub(super) struct DynIOStream<'a>(Box<dyn IOStream + 'a>);
+pub(super) struct DynIOStream<'io_stream>(Box<dyn IOStream + 'io_stream>);
 
-impl<'a> DynIOStream<'a> {
+impl<'io_stream> DynIOStream<'io_stream> {
 
     // Implement the exposed functions for cxx bridge
     pub fn name(&mut self) -> String {

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

@@ -2,16 +2,16 @@ pub use super::bridge::{CPPOpusFile, CPPVorbisFile};
 use super::xiph::XiphComment;
 use std::pin::Pin;
 
-pub struct VorbisFile<'a> {
+pub struct VorbisFile<'file_ref> {
-    this: Pin<&'a mut CPPVorbisFile>
+    this: Pin<&'file_ref mut CPPVorbisFile>
 }
 
-impl<'a> VorbisFile<'a> {
+impl<'file_ref> VorbisFile<'file_ref> {
-    pub(super) fn new(this: Pin<&'a mut CPPVorbisFile>) -> Self {
+    pub(super) fn new(this: Pin<&'file_ref mut CPPVorbisFile>) -> Self {
         Self { this }
     }
 
-    pub fn xiph_comments(&self) -> Option<XiphComment> {
+    pub fn xiph_comments(&self) -> Option<XiphComment<'file_ref>> {
         let this = self.this.as_ref();
         let tag = this.vorbisTag();
         let tag_ref = unsafe {
@@ -24,25 +24,18 @@ impl<'a> VorbisFile<'a> {
     }
 }
 
-pub struct OpusFile<'a> {
+pub struct OpusFile<'file_ref> {
-    this: Pin<&'a mut CPPOpusFile>
+    this: Pin<&'file_ref mut CPPOpusFile>
 }
 
-impl <'a> OpusFile<'a> {
+impl<'file_ref> OpusFile<'file_ref> {
-    pub(super) fn new(this: Pin<&'a mut CPPOpusFile>) -> Self {
+    pub(super) fn new(this: Pin<&'file_ref mut CPPOpusFile>) -> Self {
         Self { this }
     }
 
-    pub fn xiph_comments(&self) -> Option<XiphComment<'a>> {
+    pub fn xiph_comments(&self) -> Option<XiphComment<'file_ref>> {
         let this = self.this.as_ref();
-        let tag = unsafe {
+        let tag = this.opusTag();
-            // SAFETY:
-            // - This pin is only used in this unsafe scope.
-            // - The pin is used as a C++ this pointer in the ffi call, which does
-            //   not change address by C++ semantics.
-            // - The value is a pointer that does not depend on the address of self.
-            this.opusTag()
-        };
         let tag_ref = unsafe {
             // SAFETY: This pointer is a valid type, and can only used and accessed
             // via this function and thus cannot be mutated, satisfying the aliasing rules.

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

@@ -4,17 +4,17 @@ use std::marker::PhantomData;
 use std::pin::Pin;
 use std::{ffi::CStr, string::ToString};
 
-pub struct String<'a> {
+pub struct String<'file_ref> {
-    this: Pin<&'a CPPString>,
+    this: Pin<&'file_ref CPPString>,
 }
 
-impl<'a> String<'a> {
+impl<'file_ref> String<'file_ref> {
-    pub(super) fn new(this: Pin<&'a CPPString>) -> Self {
+    pub(super) fn new(this: Pin<&'file_ref CPPString>) -> Self {
         Self { this }
     }
 }
 
-impl<'a> ToString for String<'a> {
+impl<'file_ref> ToString for String<'file_ref> {
     fn to_string(&self) -> std::string::String {
         let c_str = self.this.toCString(true);
         unsafe {
@@ -34,12 +34,12 @@ impl<'a> ToString for String<'a> {
     }
 }
 
-pub struct StringList<'a> {
+pub struct StringList<'file_ref> {
-    this: Pin<&'a CPPStringList>,
+    this: Pin<&'file_ref CPPStringList>,
 }
 
-impl<'a> StringList<'a> {
+impl<'file_ref> StringList<'file_ref> {
-    pub(super) fn new(this: Pin<&'a CPPStringList>) -> Self {
+    pub(super) fn new(this: Pin<&'file_ref CPPStringList>) -> Self {
         Self { this }
     }
 
@@ -57,15 +57,15 @@ impl<'a> StringList<'a> {
     }
 }
 
-pub struct ByteVector<'a> {
+pub struct ByteVector<'file_ref> {
     // ByteVector is implicitly tied to the lifetime of the parent that owns it,
     // so we need to track that lifetime. Only reason why it's a UniquePtr is because
     // we can't marshal over ownership of the ByteVector by itself over cxx.
-    _data: PhantomData<&'a CPPByteVector>,
+    _data: PhantomData<&'file_ref CPPByteVector>,
     this: UniquePtr<CPPByteVector>,
 }
 
-impl<'a> ByteVector<'a> {
+impl<'file_ref> ByteVector<'file_ref> {
     pub(super) fn new(this: UniquePtr<CPPByteVector>) -> Self {
         Self {
             _data: PhantomData,

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

@@ -5,38 +5,47 @@ use super::tk;
 use std::pin::Pin;
 use std::collections::HashMap;
 
-pub struct XiphComment<'a> {
+pub struct XiphComment<'file_ref> {
-    this: Pin<&'a mut CPPXiphComment>
+    this: Pin<&'file_ref mut CPPXiphComment>
 }
 
-impl<'a> XiphComment<'a> {
+impl<'file_ref> XiphComment<'file_ref> {
-    pub(super) fn new(this: Pin<&'a mut CPPXiphComment>) -> Self {
+    pub(super) fn new(this: Pin<&'file_ref mut CPPXiphComment>) -> Self {
         Self { this }
     }
 
-    pub fn field_list_map(&'a self) -> FieldListMap<'a> {
+    pub fn field_list_map<'slf>(&'slf self) -> FieldListMap<'file_ref> {
-        let map = self.this.as_ref().fieldListMap();
+        // To call the method we need, we have to get our mut reference down to an immutable
+        // reference. The safe API can do this, but shortens the lifecycle to at most self, even
+        // though the reference really lives as long as file_ref. Sadly, this requires us to transmute 
+        // to extend the lifecycle back. This new pointer is really unsafe (we now have both a mut
+        // and an immutable reference to the same object), but it's dropped after this call.
+        // The value returned is unable to actually mutate this object, so it's safe.
+        let this_ref: &'slf CPPXiphComment = self.this.as_ref().get_ref();
+        let extended_ref: &'file_ref CPPXiphComment = unsafe { std::mem::transmute(this_ref) };
+        let this: Pin<&'file_ref CPPXiphComment> = unsafe { Pin::new_unchecked(extended_ref) };
+        let map = this.fieldListMap();
         let map_pin = unsafe { Pin::new_unchecked(map) };
         FieldListMap::new(map_pin)
     }
 
-    pub fn picture_list(&mut self) -> PictureList<'a> {
+    pub fn picture_list(&mut self) -> PictureList<'file_ref> {
         let pictures = XiphComment_pictureList(self.this.as_mut());
         PictureList::new(pictures)
     }
 }
 
-pub struct FieldListMap<'a> {
+pub struct FieldListMap<'file_ref> {
-    this: Pin<&'a CPPFieldListMap>,
+    this: Pin<&'file_ref CPPFieldListMap>,
 }
 
-impl<'a> FieldListMap<'a> {
+impl<'file_ref> FieldListMap<'file_ref> {
-    pub(super) fn new(this: Pin<&'a CPPFieldListMap>) -> Self {
+    pub(super) fn new(this: Pin<&'file_ref CPPFieldListMap>) -> Self {
         Self { this }
     }
 }
 
-impl<'a> FieldListMap<'a> {
+impl<'file_ref> FieldListMap<'file_ref> {
     pub fn to_hashmap(&self) -> HashMap<String, Vec<String>> {
         let cxx_vec = FieldListMap_to_entries(self.this);
         cxx_vec