Intern returned C strings to avoid leaking them.

capi/Cargo.toml

@@ -10,7 +10,6 @@ crate-type = ["staticlib"]
 [dependencies]
 libc = "0.2"
 eventual    = "~0.1.5"
-owning_ref = "0.1.*"
 
 [dependencies.librespot]
 path = "../"

capi/src/artist.rs

@@ -1,6 +1,4 @@
 use libc::c_char;
-use std::ffi::CString;
-use std::mem;
 
 use librespot::metadata::Artist;
 
@@ -17,17 +15,11 @@ pub unsafe extern "C" fn sp_artist_is_loaded(c_artist: *mut sp_artist) -> bool {
 
 #[no_mangle]
 pub unsafe extern "C" fn sp_artist_name(c_artist: *mut sp_artist) -> *const c_char {
-    let artist = &*c_artist;
+    let artist = &mut *c_artist;
 
     let name = artist.get()
-                     .map(|metadata| metadata.name.clone())
-                     .unwrap_or("".to_owned());
-
-    let name = CString::new(name).unwrap();
-    let c_name = name.as_ptr();
-
-    // FIXME
-    mem::forget(name);
+                     .map(|metadata| &metadata.name as &str)
+                     .unwrap_or("");
 
-    c_name
+    artist.intern(name).as_ptr()
 }

capi/src/cstring_cache.rs

@@ -0,0 +1,21 @@
+use std::collections::HashMap;
+use std::ffi::{CString, CStr};
+
+pub struct CStringCache {
+    cache: HashMap<String, CString>
+}
+
+impl CStringCache {
+    pub fn new() -> CStringCache {
+        CStringCache {
+            cache: HashMap::new()
+        }
+    }
+
+    pub fn intern(&mut self, string: &str) -> &CStr {
+        self.cache.entry(string.to_owned()).or_insert_with(|| {
+            CString::new(string).unwrap()
+        })
+    }
+}
+

capi/src/lib.rs

@@ -1,7 +1,8 @@
+#![feature(fnbox)]
+
 extern crate librespot;
 extern crate libc;
 extern crate eventual;
-extern crate owning_ref;
 
 pub mod artist;
 pub mod link;
@@ -9,9 +10,5 @@ pub mod metadata;
 pub mod session;
 pub mod track;
 mod types;
-
-pub use types::sp_session_config;
-pub use types::sp_error;
-pub use types::sp_error::*;
-
+mod cstring_cache;
 

capi/src/link.rs

@@ -1,5 +1,5 @@
 use metadata::SpMetadata;
-use session::global_session;
+use session::SpSession;
 use track::sp_track;
 use types::sp_error;
 use types::sp_error::*;
@@ -29,8 +29,8 @@ pub unsafe extern "C" fn sp_link_release(c_link: *mut sp_link) -> sp_error {
 #[no_mangle]
 pub unsafe extern "C" fn sp_link_as_track(c_link: *mut sp_link) -> *mut sp_track {
     let link = &*c_link;
-    let session = &*global_session.unwrap();
+    let session = SpSession::global();
 
-    let track = SpMetadata::from_future(link.as_track(session).unwrap());
+    let track = SpMetadata::from_future(link.as_track(&session.session).unwrap());
     Box::into_raw(Box::new(track))
 }

capi/src/metadata.rs

@@ -1,54 +1,87 @@
 use eventual::Async;
-use owning_ref::MutexGuardRef;
-use std::sync::{Mutex, Arc};
+use std::sync::Arc;
+use std::ffi::CStr;
+use std::cell::UnsafeCell;
 
 use librespot::metadata::{MetadataTrait, MetadataRef};
 
-pub enum SpMetadataInner<T: MetadataTrait> {
+use cstring_cache::CStringCache;
+
+use session::SpSession;
+
+pub struct UnsafeSyncCell<T> {
+    cell: UnsafeCell<T>
+}
+
+impl <T> UnsafeSyncCell<T> {
+    fn new(value: T) -> UnsafeSyncCell<T> {
+        UnsafeSyncCell { cell: UnsafeCell::new(value) }
+    }
+
+    fn get(&self) -> *mut T {
+        self.cell.get()
+    }
+}
+
+unsafe impl<T> Sync for UnsafeSyncCell<T> {}
+
+pub enum SpMetadataState<T: MetadataTrait> {
     Loading,
     Error,
     Loaded(T),
 }
 
-pub struct SpMetadata<T: MetadataTrait>(Arc<Mutex<SpMetadataInner<T>>>);
+pub struct SpMetadata<T: MetadataTrait> {
+    state: Arc<UnsafeSyncCell<SpMetadataState<T>>>,
+    cache: CStringCache,
+}
 
 impl <T: MetadataTrait> SpMetadata<T> {
     pub fn from_future(future: MetadataRef<T>) -> SpMetadata<T> {
-        let metadata = Arc::new(Mutex::new(SpMetadataInner::Loading));
+        let state = Arc::new(UnsafeSyncCell::new(SpMetadataState::Loading));
 
         {
-            let metadata = metadata.clone();
-            future.receive(move |result| {
-                //let metadata = metadata.upgrade().unwrap();
-                let mut metadata = metadata.lock().unwrap();
-
-                *metadata = match result {
-                    Ok(data) =>  SpMetadataInner::Loaded(data),
-                    Err(_) => SpMetadataInner::Error,
+            let state = state.clone();
+            SpSession::receive(future, move |session, result| {
+                let state = unsafe {
+                    &mut *state.get()
+                };
+
+                *state = match result {
+                    Ok(data) => SpMetadataState::Loaded(data),
+                    Err(_) => SpMetadataState::Error,
                 };
+
+                unsafe {
+                    if let Some(f) = session.callbacks.metadata_updated {
+                        f(session as *mut _)
+                    }
+                }
             });
         }
 
-        SpMetadata(metadata)
+        SpMetadata {
+            state: state,
+            cache: CStringCache::new(),
+        }
     }
 
     pub fn is_loaded(&self) -> bool {
-        self.get().is_some()
+        unsafe {
+            self.get().is_some()
+        }
     }
 
-    pub fn get(&self) -> Option<MutexGuardRef<SpMetadataInner<T>, T>> {
-        let inner = self.0.lock().unwrap();
+    pub unsafe fn get(&self) -> Option<&'static T> {
+        let state = &*self.state.get();
 
-        match *inner {
-            SpMetadataInner::Loaded(_) => {
-                Some(MutexGuardRef::new(inner).map(|inner| {
-                    match *inner {
-                        SpMetadataInner::Loaded(ref metadata) => metadata,
-                        _ => unreachable!(),
-                    }
-                }))
-            }
+        match *state {
+            SpMetadataState::Loaded(ref metadata) => Some(metadata),
             _ => None,
         }
     }
+
+    pub fn intern(&mut self, string: &str) -> &CStr {
+        self.cache.intern(string)
+    }
 }

capi/src/session.rs

@@ -1,23 +1,63 @@
 use libc::{c_int, c_char};
-use std::ffi::{CStr, CString};
-use std::mem;
+use std::ffi::CStr;
 use std::slice::from_raw_parts;
+use std::sync::mpsc;
+use std::boxed::FnBox;
+use std::sync::Mutex;
 
 use librespot::session::{Session, Config, Bitrate};
+use eventual::{Async, AsyncResult, Future};
 
+use cstring_cache::CStringCache;
 use types::sp_error;
 use types::sp_error::*;
 use types::sp_session_config;
+use types::sp_session_callbacks;
 
-pub static mut global_session: Option<*mut Session> = None;
+static mut global_session: Option<(*const sp_session, *const Mutex<mpsc::Sender<SpSessionEvent>>)> = None;
+
+pub type SpSessionEvent = Box<FnBox(&mut SpSession) -> ()>;
+
+pub struct SpSession {
+    pub session: Session,
+    cache: CStringCache,
+    rx: mpsc::Receiver<SpSessionEvent>,
+
+    pub callbacks: &'static sp_session_callbacks,
+}
+
+impl SpSession {
+    pub unsafe fn global() -> &'static SpSession {
+        &*global_session.unwrap().0
+    }
+
+    pub fn run<F: FnOnce(&mut SpSession) -> () + 'static>(event: F) {
+        let tx = unsafe {
+            &*global_session.unwrap().1
+        };
+        
+        tx.lock().unwrap().send(Box::new(event)).unwrap();
+    }
+
+    pub fn receive<T, E, F>(future: Future<T, E>, handler: F)
+        where T : Send, E: Send,
+              F : FnOnce(&mut SpSession, AsyncResult<T, E>) -> () + Send + 'static {
+
+        future.receive(move |result| {
+            SpSession::run(move |session| {
+                handler(session, result);
+            })
+        })
+    }
+}
 
 #[allow(non_camel_case_types)]
-pub type sp_session = Session;
+pub type sp_session = SpSession;
 
 #[no_mangle]
 pub unsafe extern "C" fn sp_session_create(c_config: *const sp_session_config,
                                            c_session: *mut *mut sp_session) -> sp_error {
-    assert_eq!(global_session, None);
+    assert!(global_session.is_none());
 
     let c_config = &*c_config;
 
@@ -36,10 +76,20 @@ pub unsafe extern "C" fn sp_session_create(c_config: *const sp_session_config,
         bitrate: Bitrate::Bitrate160,
     };
 
-    let session = Box::new(Session::new(config));
-    let session = Box::into_raw(session);
+    let (tx, rx) = mpsc::channel();
+
+    let session = SpSession {
+        session: Session::new(config),
+        cache: CStringCache::new(),
+        rx: rx,
+        callbacks: &*c_config.callbacks,
+    };
+
+    let session = Box::into_raw(Box::new(session));
+    let tx = Box::into_raw(Box::new(Mutex::new(tx)));
+
+    global_session = Some((session, tx));
 
-    global_session = Some(session);
     *c_session = session;
 
     SP_ERROR_OK
@@ -47,8 +97,6 @@ pub unsafe extern "C" fn sp_session_create(c_config: *const sp_session_config,
 
 #[no_mangle]
 pub unsafe extern "C" fn sp_session_release(c_session: *mut sp_session) -> sp_error {
-    assert_eq!(global_session, Some(c_session));
-
     global_session = None;
     drop(Box::from_raw(c_session));
 
@@ -61,20 +109,25 @@ pub unsafe extern "C" fn sp_session_login(c_session: *mut sp_session,
                                           c_password: *const c_char,
                                           _remember_me: bool,
                                           _blob: *const c_char) -> sp_error {
-    assert_eq!(global_session, Some(c_session));
-
     let session = &*c_session;
 
     let username = CStr::from_ptr(c_username).to_string_lossy().into_owned();
     let password = CStr::from_ptr(c_password).to_string_lossy().into_owned();
 
-    session.login_password(username, password).unwrap();
-
     {
-        let session = session.clone();
-        ::std::thread::spawn(move || {
-            loop {
-                session.poll();
+        let session = session.session.clone();
+        SpSession::receive(Future::spawn(move || {
+            session.login_password(username, password)
+        }), |session, result| {
+            result.unwrap();
+
+            {
+                let session = session.session.clone();
+                ::std::thread::spawn(move || {
+                    loop {
+                        session.poll();
+                    }
+                });
             }
         });
     }
@@ -84,27 +137,32 @@ pub unsafe extern "C" fn sp_session_login(c_session: *mut sp_session,
 
 #[no_mangle]
 pub unsafe extern "C" fn sp_session_user_name(c_session: *mut sp_session) -> *const c_char {
-    assert_eq!(global_session, Some(c_session));
-
-    let session = &*c_session;
-
-    let username = CString::new(session.username()).unwrap();
-    let c_username = username.as_ptr();
+    let session = &mut *c_session;
 
-    // FIXME
-    mem::forget(username);
-
-    c_username
+    let username = session.session.username();
+    session.cache.intern(&username).as_ptr()
 }
 
 #[no_mangle]
 pub unsafe extern "C" fn sp_session_user_country(c_session: *mut sp_session) -> c_int {
-    assert_eq!(global_session, Some(c_session));
-
     let session = &*c_session;
 
-    let country = session.username();
+    let country = session.session.country();
     country.chars().fold(0, |acc, x| {
         acc << 8 | (x as u32)
     }) as c_int
 }
+
+#[no_mangle]
+pub unsafe extern "C" fn sp_session_process_events(c_session: *mut sp_session, next_timeout: *mut c_int) -> sp_error {
+    let session = &mut *c_session;
+
+    if !next_timeout.is_null() {
+        *next_timeout = 10;
+    }
+
+    let event = session.rx.recv().unwrap();
+    event.call_box((session,));
+
+    SP_ERROR_OK
+}

capi/src/track.rs

@@ -1,11 +1,9 @@
 use libc::{c_int, c_char};
-use std::ffi::CString;
-use std::mem;
 use std::ptr::null_mut;
 
 use artist::sp_artist;
 use metadata::SpMetadata;
-use session::global_session;
+use session::SpSession;
 
 use librespot::metadata::{Track, Artist};
 
@@ -20,19 +18,13 @@ pub unsafe extern "C" fn sp_track_is_loaded(c_track: *mut sp_track) -> bool {
 
 #[no_mangle]
 pub unsafe extern "C" fn sp_track_name(c_track: *mut sp_track) -> *const c_char {
-    let track = &*c_track;
+    let track = &mut *c_track;
 
     let name = track.get()
-                    .map(|metadata| metadata.name.clone())
-                    .unwrap_or("".to_owned());
-
-    let name = CString::new(name).unwrap();
-    let c_name = name.as_ptr();
-
-    // FIXME
-    mem::forget(name);
+                    .map(|metadata| &metadata.name as &str)
+                    .unwrap_or("");
 
-    c_name
+    track.intern(name).as_ptr()
 }
 
 #[no_mangle]
@@ -47,14 +39,12 @@ pub unsafe extern "C" fn sp_track_num_artists(c_track: *mut sp_track) -> c_int {
 #[no_mangle]
 pub unsafe extern "C" fn sp_track_artist(c_track: *mut sp_track, index: c_int) -> *mut sp_artist {
     let track = &*c_track;
-    let session = &*global_session.unwrap();
+    let session = SpSession::global();
 
     track.get()
          .and_then(|metadata| metadata.artists.get(index as usize).map(|x| *x))
-         .map(|artist_id| {
-             let artist = SpMetadata::from_future(session.metadata::<Artist>(artist_id));
-             Box::into_raw(Box::new(artist))
-         })
+         .map(|artist_id| session.session.metadata::<Artist>(artist_id))
+         .map(|artist| Box::into_raw(Box::new(SpMetadata::from_future(artist))))
          .unwrap_or(null_mut())
 }
 

capi/src/types.rs

@@ -1,8 +1,7 @@
-#![allow(non_camel_case_types)]
+#![allow(non_camel_case_types, dead_code)]
 
-use libc::size_t;
-
-pub enum sp_session_callbacks {}
+use libc::{size_t, c_int, c_char, c_void};
+use session::sp_session;
 
 #[derive(Clone, Copy)]
 #[repr(u32)]
@@ -48,21 +47,102 @@ pub enum sp_error {
 #[repr(C)]
 #[derive(Copy,Clone)]
 pub struct sp_session_config {
-    pub api_version: ::std::os::raw::c_int,
-    pub cache_location: *const ::std::os::raw::c_char,
-    pub settings_location: *const ::std::os::raw::c_char,
-    pub application_key: *const ::std::os::raw::c_void,
+    pub api_version: c_int,
+    pub cache_location: *const c_char,
+    pub settings_location: *const c_char,
+    pub application_key: *const c_void,
     pub application_key_size: size_t,
-    pub user_agent: *const ::std::os::raw::c_char,
+    pub user_agent: *const c_char,
     pub callbacks: *const sp_session_callbacks,
-    pub userdata: *mut ::std::os::raw::c_void,
+    pub userdata: *mut c_void,
     pub compress_playlists: bool,
     pub dont_save_metadata_for_playlists: bool,
     pub initially_unload_playlists: bool,
-    pub device_id: *const ::std::os::raw::c_char,
-    pub proxy: *const ::std::os::raw::c_char,
-    pub proxy_username: *const ::std::os::raw::c_char,
-    pub proxy_password: *const ::std::os::raw::c_char,
-    pub tracefile: *const ::std::os::raw::c_char,
+    pub device_id: *const c_char,
+    pub proxy: *const c_char,
+    pub proxy_username: *const c_char,
+    pub proxy_password: *const c_char,
+    pub tracefile: *const c_char,
+}
+
+#[repr(C)]
+#[derive(Clone, Copy)]
+pub struct sp_session_callbacks {
+    pub logged_in: Option<unsafe extern "C" fn(session: *mut sp_session,
+                                               error: sp_error)>,
+
+    pub logged_out: Option<unsafe extern "C" fn(session: *mut sp_session)>,
+
+    pub metadata_updated: Option<unsafe extern "C" fn(session: *mut sp_session)>,
+
+    pub connection_error: Option<unsafe extern "C" fn(session: *mut sp_session,
+                                                      error: sp_error)>,
+
+    pub message_to_user: Option<unsafe extern "C" fn(session: *mut sp_session,
+                                                     message: *const c_char)>,
+
+    pub notify_main_thread: Option<unsafe extern "C" fn(session: *mut sp_session)>,
+
+    pub music_delivery: Option<unsafe extern "C" fn(session: *mut sp_session,
+                                                                   format: *const sp_audioformat,
+                                                                   frames: *const c_void,
+                                                                   num_frames: c_int)
+                                                                   -> c_int>,
+
+    pub play_token_lost: Option<unsafe extern "C" fn(session: *mut sp_session)>,
+
+    pub log_message: Option<unsafe extern "C" fn(session: *mut sp_session,
+                                                                data: *const c_char)>,
+
+    pub end_of_track: Option<unsafe extern "C" fn(session: *mut sp_session)>,
+
+    pub streaming_error: Option<unsafe extern "C" fn(session: *mut sp_session,
+                                                                    error: sp_error)>,
+
+    pub userinfo_updated: Option<unsafe extern "C" fn(session: *mut sp_session)>,
+
+    pub start_playback: Option<unsafe extern "C" fn(session: *mut sp_session)>,
+
+    pub stop_playback: Option<unsafe extern "C" fn(session: *mut sp_session)>,
+
+    pub get_audio_buffer_stats: Option<unsafe extern "C" fn(session: *mut sp_session,
+                                                            stats: *mut sp_audio_buffer_stats)>,
+
+    pub offline_status_updated: Option<unsafe extern "C" fn(session: *mut sp_session)>,
+
+    pub offline_error: Option<unsafe extern "C" fn(session: *mut sp_session,
+                                                   error: sp_error)>,
+
+    pub credentials_blob_updated: Option<unsafe extern "C" fn(session: *mut sp_session,
+                                                              blob: *const c_char)>,
+
+    pub connectionstate_updated: Option<unsafe extern "C" fn(session: *mut sp_session)>,
+
+    pub scrobble_error: Option<unsafe extern "C" fn(session: *mut sp_session,
+                                                    error: sp_error)>,
+
+    pub private_session_mode_changed: Option<unsafe extern "C" fn(session: *mut sp_session,
+                                                                  is_private: bool)>,
+}
+
+#[repr(C)]
+#[derive(Clone, Copy)]
+pub struct sp_audioformat {
+    pub sample_type: sp_sampletype,
+    pub sample_rate: c_int,
+    pub channels: c_int,
 }
 
+#[derive(Clone, Copy)]
+#[repr(u32)]
+pub enum sp_sampletype {
+    SP_SAMPLETYPE_INT16_NATIVE_ENDIAN = 0,
+    _Dummy // rust #10292
+}
+
+#[repr(C)]
+#[derive(Clone, Copy)]
+pub struct sp_audio_buffer_stats {
+    pub samples: c_int,
+    pub stutter: c_int,
+}