From 78f7477d8738868271c421233b42ab412add02cb Mon Sep 17 00:00:00 2001 From: Ryo Yamashita Date: Fri, 16 Jun 2023 00:21:24 +0900 Subject: [PATCH 01/13] =?UTF-8?q?"buffer"=E3=82=92Rust=E3=81=AE=E4=B8=96?= =?UTF-8?q?=E7=95=8C=E3=81=A7=E4=BF=9D=E6=8C=81=E3=81=97=E7=B6=9A=E3=81=91?= =?UTF-8?q?=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/voicevox_core_c_api/src/helpers.rs | 193 ------------------ crates/voicevox_core_c_api/src/lib.rs | 36 ++-- crates/voicevox_core_c_api/src/owners.rs | 4 + .../src/owners/c_string.rs | 103 ++++++++++ .../voicevox_core_c_api/src/owners/slice.rs | 92 +++++++++ 5 files changed, 216 insertions(+), 212 deletions(-) create mode 100644 crates/voicevox_core_c_api/src/owners.rs create mode 100644 crates/voicevox_core_c_api/src/owners/c_string.rs create mode 100644 crates/voicevox_core_c_api/src/owners/slice.rs diff --git a/crates/voicevox_core_c_api/src/helpers.rs b/crates/voicevox_core_c_api/src/helpers.rs index 406ac15c7..c62fdeb83 100644 --- a/crates/voicevox_core_c_api/src/helpers.rs +++ b/crates/voicevox_core_c_api/src/helpers.rs @@ -1,5 +1,3 @@ -use std::alloc::Layout; -use std::collections::{BTreeMap, BTreeSet}; use std::fmt::Debug; use const_default::ConstDefault; @@ -177,194 +175,3 @@ impl ConstDefault for VoicevoxSynthesisOptions { } }; } - -// libcのmallocで追加のアロケーションを行うことなく、`Vec`や`Vec`の内容を直接Cの世界に貸し出す。 - -/// Rustの世界の`Box<[impl Copy]>`をCの世界に貸し出すため、アドレスとレイアウトを管理するもの。 -/// -/// `Mutex`による内部可変性を持ち、すべての操作は`&self`から行うことができる。 -pub(crate) struct BufferManager(Mutex); - -struct BufferManagerInner { - address_to_layout_table: BTreeMap, - owned_str_addrs: BTreeSet, - static_str_addrs: BTreeSet, -} - -impl BufferManager { - pub const fn new() -> Self { - Self(Mutex::new(BufferManagerInner { - address_to_layout_table: BTreeMap::new(), - owned_str_addrs: BTreeSet::new(), - static_str_addrs: BTreeSet::new(), - })) - } - - pub fn vec_into_raw(&self, vec: Vec) -> (*mut T, usize) { - let BufferManagerInner { - address_to_layout_table, - .. - } = &mut *self.0.lock().unwrap(); - - let slice = Box::leak(vec.into_boxed_slice()); - let layout = Layout::for_value(slice); - let len = slice.len(); - let ptr = slice.as_mut_ptr(); - let addr = ptr as usize; - - let not_occupied = address_to_layout_table.insert(addr, layout).is_none(); - - assert!(not_occupied, "すでに値が入っている状態はおかしい"); - - (ptr, len) - } - - /// `vec_into_raw`でC API利用側に貸し出したポインタに対し、デアロケートする。 - /// - /// # Safety - /// - /// - `buffer_ptr`は`vec_into_raw`で取得したものであること。 - pub unsafe fn dealloc_slice(&self, buffer_ptr: *const T) { - let BufferManagerInner { - address_to_layout_table, - .. - } = &mut *self.0.lock().unwrap(); - - let addr = buffer_ptr as usize; - let layout = address_to_layout_table.remove(&addr).expect( - "解放しようとしたポインタはvoicevox_coreの管理下にありません。\ - 誤ったポインタであるか、二重解放になっていることが考えられます", - ); - - if layout.size() > 0 { - // `T: Copy`より、`T: !Drop`であるため`drop_in_place`は不要 - - // SAFETY: - // - `addr`と`layout`は対応したものである - // - `layout.size() > 0`より、`addr`はダングリングではない有効なポインタである - std::alloc::dealloc(addr as *mut u8, layout); - } - } - - pub fn c_string_into_raw(&self, s: CString) -> *mut c_char { - let BufferManagerInner { - owned_str_addrs, .. - } = &mut *self.0.lock().unwrap(); - - let ptr = s.into_raw(); - owned_str_addrs.insert(ptr as _); - ptr - } - - /// `c_string_into_raw`でC API利用側に貸し出したポインタに対し、デアロケートする。 - /// - /// # Safety - /// - /// - `ptr`は`c_string_into_raw`で取得したものであること。 - pub unsafe fn dealloc_c_string(&self, ptr: *mut c_char) { - let BufferManagerInner { - owned_str_addrs, - static_str_addrs, - .. - } = &mut *self.0.lock().unwrap(); - - if !owned_str_addrs.remove(&(ptr as _)) { - if static_str_addrs.contains(&(ptr as _)) { - panic!( - "解放しようとしたポインタはvoicevox_core管理下のものですが、\ - voicevox_coreがアンロードされるまで永続する文字列に対するものです。\ - 解放することはできません", - ) - } - panic!( - "解放しようとしたポインタはvoicevox_coreの管理下にありません。\ - 誤ったポインタであるか、二重解放になっていることが考えられます", - ); - } - drop(CString::from_raw(ptr)); - } - - pub fn memorize_static_str(&self, ptr: *const c_char) -> *const c_char { - let BufferManagerInner { - static_str_addrs, .. - } = &mut *self.0.lock().unwrap(); - - static_str_addrs.insert(ptr as _); - ptr - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn buffer_manager_works() { - let mut buffer_manager = BufferManager::new(); - - rent_and_dealloc(&mut buffer_manager, vec::<()>(0, &[])); - rent_and_dealloc(&mut buffer_manager, vec(0, &[()])); - rent_and_dealloc(&mut buffer_manager, vec(2, &[()])); - - rent_and_dealloc(&mut buffer_manager, vec::(0, &[])); - rent_and_dealloc(&mut buffer_manager, vec(0, &[0u8])); - rent_and_dealloc(&mut buffer_manager, vec(2, &[0u8])); - - rent_and_dealloc(&mut buffer_manager, vec::(0, &[])); - rent_and_dealloc(&mut buffer_manager, vec(0, &[0f32])); - rent_and_dealloc(&mut buffer_manager, vec(2, &[0f32])); - - fn rent_and_dealloc(buffer_manager: &mut BufferManager, vec: Vec) { - let expected_len = vec.len(); - let (ptr, len) = buffer_manager.vec_into_raw(vec); - assert_eq!(expected_len, len); - unsafe { - buffer_manager.dealloc_slice(ptr); - } - } - - fn vec(initial_cap: usize, elems: &[T]) -> Vec { - let mut vec = Vec::with_capacity(initial_cap); - vec.extend_from_slice(elems); - vec - } - } - - #[test] - #[should_panic( - expected = "解放しようとしたポインタはvoicevox_coreの管理下にありません。誤ったポインタであるか、二重解放になっていることが考えられます" - )] - fn buffer_manager_denies_unknown_slice_ptr() { - let buffer_manager = BufferManager::new(); - unsafe { - let x = 42; - buffer_manager.dealloc_slice(&x as *const i32); - } - } - - #[test] - #[should_panic( - expected = "解放しようとしたポインタはvoicevox_coreの管理下にありません。誤ったポインタであるか、二重解放になっていることが考えられます" - )] - fn buffer_manager_denies_unknown_char_ptr() { - let buffer_manager = BufferManager::new(); - unsafe { - let s = CStr::from_bytes_with_nul(b"\0").unwrap().to_owned(); - buffer_manager.dealloc_c_string(s.into_raw()); - } - } - - #[test] - #[should_panic( - expected = "解放しようとしたポインタはvoicevox_core管理下のものですが、voicevox_coreがアンロードされるまで永続する文字列に対するものです。解放することはできません" - )] - fn buffer_manager_denies_known_static_char_ptr() { - let buffer_manager = BufferManager::new(); - unsafe { - buffer_manager.memorize_static_str(STATIC.as_ptr() as _); - buffer_manager.dealloc_c_string(STATIC.as_ptr() as *mut c_char); - } - - static STATIC: &CStr = unsafe { CStr::from_bytes_with_nul_unchecked(b"\0") }; - } -} diff --git a/crates/voicevox_core_c_api/src/lib.rs b/crates/voicevox_core_c_api/src/lib.rs index 052b5e490..74ae3948f 100644 --- a/crates/voicevox_core_c_api/src/lib.rs +++ b/crates/voicevox_core_c_api/src/lib.rs @@ -2,7 +2,9 @@ mod c_impls; /// cbindgen:ignore mod compatible_engine; mod helpers; +mod owners; use self::helpers::*; +use self::owners::{C_STRING_OWNER, U8_SLICE_OWNER}; use chrono::SecondsFormat; use const_default::ConstDefault; use derive_getters::Getters; @@ -65,9 +67,6 @@ static RUNTIME: Lazy = Lazy::new(|| { Runtime::new().unwrap() }); -// C_APIに渡すために，VecやCStringのサイズを記憶しながら生ポインタを得るためのマネージャ -static BUFFER_MANAGER: BufferManager = BufferManager::new(); - /* * Cの関数として公開するための型や関数を定義するこれらの実装はvoicevox_core/publish.rsに定義してある対応する関数にある * この関数ではvoicevox_core/publish.rsにある対応する関数の呼び出しと、その戻り値をCの形式に変換する処理のみとする @@ -356,7 +355,7 @@ pub extern "C" fn voicevox_create_supported_devices_json( into_result_code_with_error((|| { let supported_devices = CString::new(SupportedDevices::create()?.to_json().to_string()).unwrap(); - output_supported_devices_json.write(BUFFER_MANAGER.c_string_into_raw(supported_devices)); + C_STRING_OWNER.own_and_lend(supported_devices, output_supported_devices_json); Ok(()) })()) } @@ -401,7 +400,7 @@ pub unsafe extern "C" fn voicevox_synthesizer_audio_query( ))?; let audio_query = CString::new(audio_query_model_to_json(&audio_query)) .expect("should not contain '\\0'"); - output_audio_query_json.write(BUFFER_MANAGER.c_string_into_raw(audio_query)); + C_STRING_OWNER.own_and_lend(audio_query, output_audio_query_json); Ok(()) })()) } @@ -444,7 +443,7 @@ pub unsafe extern "C" fn voicevox_synthesizer_create_accent_phrases( ))?; let accent_phrases = CString::new(accent_phrases_to_json(&accent_phrases)) .expect("should not contain '\\0'"); - output_accent_phrases_json.write(BUFFER_MANAGER.c_string_into_raw(accent_phrases)); + C_STRING_OWNER.own_and_lend(accent_phrases, output_accent_phrases_json); Ok(()) })()) } @@ -476,7 +475,7 @@ pub unsafe extern "C" fn voicevox_synthesizer_replace_mora_data( )?; let accent_phrases = CString::new(accent_phrases_to_json(&accent_phrases)) .expect("should not contain '\\0'"); - output_accent_phrases_json.write(BUFFER_MANAGER.c_string_into_raw(accent_phrases)); + C_STRING_OWNER.own_and_lend(accent_phrases, output_accent_phrases_json); Ok(()) })()) } @@ -508,7 +507,7 @@ pub unsafe extern "C" fn voicevox_synthesizer_replace_phoneme_length( )?; let accent_phrases = CString::new(accent_phrases_to_json(&accent_phrases)) .expect("should not contain '\\0'"); - output_accent_phrases_json.write(BUFFER_MANAGER.c_string_into_raw(accent_phrases)); + C_STRING_OWNER.own_and_lend(accent_phrases, output_accent_phrases_json); Ok(()) })()) } @@ -540,7 +539,7 @@ pub unsafe extern "C" fn voicevox_synthesizer_replace_mora_pitch( )?; let accent_phrases = CString::new(accent_phrases_to_json(&accent_phrases)) .expect("should not contain '\\0'"); - output_accent_phrases_json.write(BUFFER_MANAGER.c_string_into_raw(accent_phrases)); + C_STRING_OWNER.own_and_lend(accent_phrases, output_accent_phrases_json); Ok(()) })()) } @@ -588,9 +587,7 @@ pub unsafe extern "C" fn voicevox_synthesizer_synthesis( StyleId::new(style_id), &SynthesisOptions::from(options), ))?; - let (ptr, len) = BUFFER_MANAGER.vec_into_raw(wav); - output_wav.write(ptr); - output_wav_length.write(len); + U8_SLICE_OWNER.own_and_lend(wav, output_wav, output_wav_length); Ok(()) })()) } @@ -636,9 +633,7 @@ pub unsafe extern "C" fn voicevox_synthesizer_tts( StyleId::new(style_id), &TtsOptions::from(options), ))?; - let (ptr, size) = BUFFER_MANAGER.vec_into_raw(output); - output_wav.write(ptr); - output_wav_length.write(size); + U8_SLICE_OWNER.own_and_lend(output, output_wav, output_wav_length); Ok(()) })()) } @@ -650,7 +645,7 @@ pub unsafe extern "C" fn voicevox_synthesizer_tts( /// @param voicevox_audio_query で確保されたポインタであり、かつ呼び出し側でバッファの変更を行われていないこと #[no_mangle] pub unsafe extern "C" fn voicevox_json_free(json: *mut c_char) { - BUFFER_MANAGER.dealloc_c_string(json); + C_STRING_OWNER.delete(json); } /// wav データのメモリを解放する @@ -660,7 +655,7 @@ pub unsafe extern "C" fn voicevox_json_free(json: *mut c_char) { /// @param wav voicevox_tts,voicevox_synthesis で確保されたポインタであり、かつ呼び出し側でバッファの変更を行われていないこと #[no_mangle] pub unsafe extern "C" fn voicevox_wav_free(wav: *mut u8) { - BUFFER_MANAGER.dealloc_slice(wav); + U8_SLICE_OWNER.delete(wav); } /// エラー結果をメッセージに変換する @@ -670,8 +665,11 @@ pub unsafe extern "C" fn voicevox_wav_free(wav: *mut u8) { pub extern "C" fn voicevox_error_result_to_message( result_code: VoicevoxResultCode, ) -> *const c_char { - BUFFER_MANAGER.memorize_static_str( - voicevox_core::result_code::error_result_to_message(result_code).as_ptr() as *const c_char, + C_STRING_OWNER.memorize_static( + CStr::from_bytes_with_nul( + voicevox_core::result_code::error_result_to_message(result_code).as_ref(), + ) + .unwrap(), ) } diff --git a/crates/voicevox_core_c_api/src/owners.rs b/crates/voicevox_core_c_api/src/owners.rs new file mode 100644 index 000000000..69360f260 --- /dev/null +++ b/crates/voicevox_core_c_api/src/owners.rs @@ -0,0 +1,4 @@ +mod c_string; +mod slice; + +pub(crate) use self::{c_string::C_STRING_OWNER, slice::U8_SLICE_OWNER}; diff --git a/crates/voicevox_core_c_api/src/owners/c_string.rs b/crates/voicevox_core_c_api/src/owners/c_string.rs new file mode 100644 index 000000000..38024f22d --- /dev/null +++ b/crates/voicevox_core_c_api/src/owners/c_string.rs @@ -0,0 +1,103 @@ +use std::{ + cell::UnsafeCell, + collections::{BTreeMap, BTreeSet}, + ffi::{c_char, CStr, CString}, + mem::MaybeUninit, + sync::Mutex, +}; + +pub(crate) static C_STRING_OWNER: CStringOwner = CStringOwner::new(); + +pub(crate) struct CStringOwner(Mutex); + +struct CStringOwnerInner { + owned_c_strings: BTreeMap>, + static_str_addrs: BTreeSet, +} + +impl CStringOwner { + const fn new() -> Self { + Self(Mutex::new(CStringOwnerInner { + owned_c_strings: BTreeMap::new(), + static_str_addrs: BTreeSet::new(), + })) + } + + pub(crate) fn own_and_lend(&self, s: CString, out: &mut MaybeUninit<*mut c_char>) { + let CStringOwnerInner { + owned_c_strings, .. + } = &mut *self.0.lock().unwrap(); + + let ptr = s.as_ptr() as *mut c_char; + + owned_c_strings.insert(ptr as usize, s.into()); + out.write(ptr); + } + + /// `c_string_into_raw`でC API利用側に貸し出したポインタに対し、デアロケートする。 + /// + /// # Safety + /// + /// - `ptr`は`c_string_into_raw`で取得したものであること。 + pub(crate) fn delete(&self, ptr: *mut c_char) { + let CStringOwnerInner { + owned_c_strings, + static_str_addrs, + .. + } = &mut *self.0.lock().unwrap(); + + let addr = ptr as usize; + if owned_c_strings.remove(&addr).is_none() { + if static_str_addrs.contains(&addr) { + panic!( + "解放しようとしたポインタはvoicevox_core管理下のものですが、\ + voicevox_coreがアンロードされるまで永続する文字列に対するものです。\ + 解放することはできません", + ) + } + panic!( + "解放しようとしたポインタはvoicevox_coreの管理下にありません。\ + 誤ったポインタであるか、二重解放になっていることが考えられます", + ); + } + } + + pub(crate) fn memorize_static(&self, s: &'static CStr) -> *const c_char { + let CStringOwnerInner { + static_str_addrs, .. + } = &mut *self.0.lock().unwrap(); + + let ptr = s.as_ptr(); + static_str_addrs.insert(ptr as usize); + ptr + } +} + +#[cfg(test)] +mod tests { + use std::ffi::{c_char, CStr}; + + use super::CStringOwner; + + #[test] + #[should_panic( + expected = "解放しようとしたポインタはvoicevox_coreの管理下にありません。誤ったポインタであるか、二重解放になっていることが考えられます" + )] + fn it_denies_unknown_char_ptr() { + let owner = CStringOwner::new(); + let s = CStr::from_bytes_with_nul(b"\0").unwrap().to_owned(); + owner.delete(s.into_raw()); + } + + #[test] + #[should_panic( + expected = "解放しようとしたポインタはvoicevox_core管理下のものですが、voicevox_coreがアンロードされるまで永続する文字列に対するものです。解放することはできません" + )] + fn it_denies_known_static_char_ptr() { + let owner = CStringOwner::new(); + owner.memorize_static(STATIC); + owner.delete(STATIC.as_ptr() as *mut c_char); + + static STATIC: &CStr = unsafe { CStr::from_bytes_with_nul_unchecked(b"\0") }; + } +} diff --git a/crates/voicevox_core_c_api/src/owners/slice.rs b/crates/voicevox_core_c_api/src/owners/slice.rs new file mode 100644 index 000000000..5c3953dda --- /dev/null +++ b/crates/voicevox_core_c_api/src/owners/slice.rs @@ -0,0 +1,92 @@ +use std::{cell::UnsafeCell, collections::BTreeMap, mem::MaybeUninit, sync::Mutex}; + +pub(crate) static U8_SLICE_OWNER: SliceOwner = SliceOwner::new(); + +pub(crate) struct SliceOwner { + slices: Mutex>>>, +} + +impl SliceOwner { + const fn new() -> Self { + Self { + slices: Mutex::new(BTreeMap::new()), + } + } + + pub(crate) fn own_and_lend( + &self, + slice: impl Into>, + out_ptr: &mut MaybeUninit<*mut T>, + out_len: &mut MaybeUninit, + ) { + let mut slices = self.slices.lock().unwrap(); + + let slice = slice.into(); + let ptr = slice.as_ptr() as *mut T; + let len = slice.len(); + + slices.insert(ptr as usize, slice.into()); + out_ptr.write(ptr); + out_len.write(len); + } + + pub(crate) fn delete(&self, ptr: *mut T) { + let mut slices = self.slices.lock().unwrap(); + + slices.remove(&(ptr as usize)).expect( + "解放しようとしたポインタはvoicevox_coreの管理下にありません。\ + 誤ったポインタであるか、二重解放になっていることが考えられます", + ); + } +} + +#[cfg(test)] +mod tests { + use std::mem::MaybeUninit; + + use super::SliceOwner; + + #[test] + fn it_works() { + lend_and_delete(vec::<()>(0, &[])); + lend_and_delete(vec(0, &[()])); + lend_and_delete(vec(2, &[()])); + + lend_and_delete(vec::(0, &[])); + lend_and_delete(vec(0, &[0u8])); + lend_and_delete(vec(2, &[0u8])); + + lend_and_delete(vec::(0, &[])); + lend_and_delete(vec(0, &[0f32])); + lend_and_delete(vec(2, &[0f32])); + + fn lend_and_delete(vec: Vec) { + let owner = SliceOwner::::new(); + let expected_len = vec.len(); + let (ptr, len) = unsafe { + let mut ptr = MaybeUninit::uninit(); + let mut len = MaybeUninit::uninit(); + owner.own_and_lend(vec, &mut ptr, &mut len); + (ptr.assume_init(), len.assume_init()) + }; + assert_eq!(expected_len, len); + owner.delete(ptr); + } + + fn vec(initial_cap: usize, elems: &[T]) -> Vec { + let mut vec = Vec::with_capacity(initial_cap); + vec.extend_from_slice(elems); + vec + } + } + + #[test] + #[should_panic( + expected = "解放しようとしたポインタはvoicevox_coreの管理下にありません。誤ったポインタであるか、二重解放になっていることが考えられます" + )] + fn it_denies_unknown_ptr() { + let owner = SliceOwner::::new(); + let x = 42; + owner.delete(&x as *const i32 as *mut i32); + } +} From 1557e7d555a56d4544a814592b4632b622e97de6 Mon Sep 17 00:00:00 2001 From: Ryo Yamashita Date: Fri, 16 Jun 2023 01:25:22 +0900 Subject: [PATCH 02/13] =?UTF-8?q?=E6=8A=9C=E3=81=91=E3=81=A6=E3=81=84?= =?UTF-8?q?=E3=81=9F=E3=80=81=E4=BA=8C=E9=87=8D=E7=99=BB=E9=8C=B2=E3=81=AB?= =?UTF-8?q?=E5=AF=BE=E3=81=99=E3=82=8Bassert=E3=82=92=E5=85=A5=E3=82=8C?= =?UTF-8?q?=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/voicevox_core_c_api/src/owners/c_string.rs | 4 +++- crates/voicevox_core_c_api/src/owners/slice.rs | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/voicevox_core_c_api/src/owners/c_string.rs b/crates/voicevox_core_c_api/src/owners/c_string.rs index 38024f22d..eacc172e4 100644 --- a/crates/voicevox_core_c_api/src/owners/c_string.rs +++ b/crates/voicevox_core_c_api/src/owners/c_string.rs @@ -30,7 +30,9 @@ impl CStringOwner { let ptr = s.as_ptr() as *mut c_char; - owned_c_strings.insert(ptr as usize, s.into()); + let duplicated = owned_c_strings.insert(ptr as usize, s.into()).is_some(); + assert!(!duplicated, "duplicated"); + out.write(ptr); } diff --git a/crates/voicevox_core_c_api/src/owners/slice.rs b/crates/voicevox_core_c_api/src/owners/slice.rs index 5c3953dda..54ec5c250 100644 --- a/crates/voicevox_core_c_api/src/owners/slice.rs +++ b/crates/voicevox_core_c_api/src/owners/slice.rs @@ -25,7 +25,9 @@ impl SliceOwner { let ptr = slice.as_ptr() as *mut T; let len = slice.len(); - slices.insert(ptr as usize, slice.into()); + let duplicated = slices.insert(ptr as usize, slice.into()).is_some(); + assert!(!duplicated, "duplicated"); + out_ptr.write(ptr); out_len.write(len); } From 5e55c54f3d240b90dcfbcb304a0dce6de367f94e Mon Sep 17 00:00:00 2001 From: Ryo Yamashita Date: Fri, 16 Jun 2023 00:45:05 +0900 Subject: [PATCH 03/13] =?UTF-8?q?=E4=B8=8D=E8=A6=81=E3=81=A8=E3=81=AA?= =?UTF-8?q?=E3=81=A3=E3=81=9F`unsafe`=E3=82=92=E5=A4=96=E3=81=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/voicevox_core_c_api/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/voicevox_core_c_api/src/lib.rs b/crates/voicevox_core_c_api/src/lib.rs index 74ae3948f..08c844bef 100644 --- a/crates/voicevox_core_c_api/src/lib.rs +++ b/crates/voicevox_core_c_api/src/lib.rs @@ -644,7 +644,7 @@ pub unsafe extern "C" fn voicevox_synthesizer_tts( /// # Safety /// @param voicevox_audio_query で確保されたポインタであり、かつ呼び出し側でバッファの変更を行われていないこと #[no_mangle] -pub unsafe extern "C" fn voicevox_json_free(json: *mut c_char) { +pub extern "C" fn voicevox_json_free(json: *mut c_char) { C_STRING_OWNER.delete(json); } @@ -654,7 +654,7 @@ pub unsafe extern "C" fn voicevox_json_free(json: *mut c_char) { /// # Safety /// @param wav voicevox_tts,voicevox_synthesis で確保されたポインタであり、かつ呼び出し側でバッファの変更を行われていないこと #[no_mangle] -pub unsafe extern "C" fn voicevox_wav_free(wav: *mut u8) { +pub extern "C" fn voicevox_wav_free(wav: *mut u8) { U8_SLICE_OWNER.delete(wav); } From 263caa9a0cbb2f672ac7b104dceae9a272c6178c Mon Sep 17 00:00:00 2001 From: Ryo Yamashita Date: Fri, 16 Jun 2023 01:14:36 +0900 Subject: [PATCH 04/13] =?UTF-8?q?docstring=E3=82=92=E6=9B=B8=E3=81=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/voicevox_core_c_api/src/owners.rs | 4 ++++ crates/voicevox_core_c_api/src/owners/c_string.rs | 13 ++++++++++--- crates/voicevox_core_c_api/src/owners/slice.rs | 9 +++++++++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/crates/voicevox_core_c_api/src/owners.rs b/crates/voicevox_core_c_api/src/owners.rs index 69360f260..639002858 100644 --- a/crates/voicevox_core_c_api/src/owners.rs +++ b/crates/voicevox_core_c_api/src/owners.rs @@ -1,3 +1,7 @@ +//! `Box<[u8]>`や`CString`といったバッファの所有者(owner)。 +//! +//! libcのmallocで追加のアロケーションを行うことなく、バッファを直接Cの世界に貸し出すことができる。 + mod c_string; mod slice; diff --git a/crates/voicevox_core_c_api/src/owners/c_string.rs b/crates/voicevox_core_c_api/src/owners/c_string.rs index eacc172e4..ccd4e5efc 100644 --- a/crates/voicevox_core_c_api/src/owners/c_string.rs +++ b/crates/voicevox_core_c_api/src/owners/c_string.rs @@ -10,6 +10,9 @@ pub(crate) static C_STRING_OWNER: CStringOwner = CStringOwner::new(); pub(crate) struct CStringOwner(Mutex); +/// Cの世界に貸し出す`CStr`の所有者。 +/// +/// `Mutex`による内部可変性を持ち、すべての操作は共有参照から行うことができる。 struct CStringOwnerInner { owned_c_strings: BTreeMap>, static_str_addrs: BTreeSet, @@ -23,6 +26,7 @@ impl CStringOwner { })) } + /// `CString`を所有し、そのポインタと長さを参照としてC API利用者に与える。 pub(crate) fn own_and_lend(&self, s: CString, out: &mut MaybeUninit<*mut c_char>) { let CStringOwnerInner { owned_c_strings, .. @@ -36,11 +40,11 @@ impl CStringOwner { out.write(ptr); } - /// `c_string_into_raw`でC API利用側に貸し出したポインタに対し、デアロケートする。 + /// `own_and_lend`でC API利用者に貸し出したポインタに対応する`CString`をデストラクトする。 /// - /// # Safety + /// # Panics /// - /// - `ptr`は`c_string_into_raw`で取得したものであること。 + /// `ptr`が`own_and_lend`で貸し出されたポインタではないとき、パニックする。 pub(crate) fn delete(&self, ptr: *mut c_char) { let CStringOwnerInner { owned_c_strings, @@ -64,6 +68,9 @@ impl CStringOwner { } } + /// `&'static CStr`のアドレスを記憶する。 + /// + /// 記憶したポインタは`delete`でのパニックメッセージの切り替えに使われる。 pub(crate) fn memorize_static(&self, s: &'static CStr) -> *const c_char { let CStringOwnerInner { static_str_addrs, .. diff --git a/crates/voicevox_core_c_api/src/owners/slice.rs b/crates/voicevox_core_c_api/src/owners/slice.rs index 54ec5c250..3df0fd7bb 100644 --- a/crates/voicevox_core_c_api/src/owners/slice.rs +++ b/crates/voicevox_core_c_api/src/owners/slice.rs @@ -1,5 +1,8 @@ use std::{cell::UnsafeCell, collections::BTreeMap, mem::MaybeUninit, sync::Mutex}; +/// Cの世界に貸し出す`[u8]`の所有者。 +/// +/// `Mutex`による内部可変性を持ち、すべての操作は共有参照から行うことができる。 pub(crate) static U8_SLICE_OWNER: SliceOwner = SliceOwner::new(); pub(crate) struct SliceOwner { @@ -13,6 +16,7 @@ impl SliceOwner { } } + /// `Box<[T]>`を所有し、その先頭ポインタと長さを参照としてC API利用者に与える。 pub(crate) fn own_and_lend( &self, slice: impl Into>, @@ -32,6 +36,11 @@ impl SliceOwner { out_len.write(len); } + /// `own_and_lend`でC API利用者に貸し出したポインタに対応する`Box<[u8]>`をデストラクトする。 + /// + /// # Panics + /// + /// `ptr`が`own_and_lend`で貸し出されたポインタではないとき、パニックする。 pub(crate) fn delete(&self, ptr: *mut T) { let mut slices = self.slices.lock().unwrap(); From 908ae9ce14fd1bf883de86b038171c315dff0bd9 Mon Sep 17 00:00:00 2001 From: Ryo Yamashita Date: Fri, 16 Jun 2023 01:36:13 +0900 Subject: [PATCH 05/13] =?UTF-8?q?doc=E3=82=92=E4=BF=AE=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/voicevox_core_c_api/src/owners/c_string.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/voicevox_core_c_api/src/owners/c_string.rs b/crates/voicevox_core_c_api/src/owners/c_string.rs index ccd4e5efc..7244117b5 100644 --- a/crates/voicevox_core_c_api/src/owners/c_string.rs +++ b/crates/voicevox_core_c_api/src/owners/c_string.rs @@ -26,7 +26,7 @@ impl CStringOwner { })) } - /// `CString`を所有し、そのポインタと長さを参照としてC API利用者に与える。 + /// `CString`を所有し、そのポインタを参照としてC API利用者に与える。 pub(crate) fn own_and_lend(&self, s: CString, out: &mut MaybeUninit<*mut c_char>) { let CStringOwnerInner { owned_c_strings, .. From 44bbe5b8947fe693e7abb50342572126c5de0fc9 Mon Sep 17 00:00:00 2001 From: Ryo Yamashita Date: Fri, 16 Jun 2023 02:08:25 +0900 Subject: [PATCH 06/13] =?UTF-8?q?doc=E3=82=92=E6=9B=B4=E6=96=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/voicevox_core_c_api/src/owners.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/voicevox_core_c_api/src/owners.rs b/crates/voicevox_core_c_api/src/owners.rs index 639002858..5318c22f7 100644 --- a/crates/voicevox_core_c_api/src/owners.rs +++ b/crates/voicevox_core_c_api/src/owners.rs @@ -1,6 +1,10 @@ //! `Box<[u8]>`や`CString`といったバッファの所有者(owner)。 //! -//! libcのmallocで追加のアロケーションを行うことなく、バッファを直接Cの世界に貸し出すことができる。 +//! 本クレートが提供するAPIとして、バイト列/文字列の生成(create)とその解放(free)がある。 +//! APIとしては"生成"時に`Box<[u8]>`/`CString`のownershipがC側に渡され、"解放"時にはそのownershipがRust側に返されるといった形となる。 +//! +//! しかし実装としては`Box`の場合とは異なり、何かしらの情報をRust側で保持し続けなくてはならない。 +//! 実態としてはRust側がバッファの所有者(owner)であり続け、C側にはその参照が渡される形になる。 mod c_string; mod slice; From 12f6d351a958f57197d6cd849a80c58d9ec7fabf Mon Sep 17 00:00:00 2001 From: Ryo Yamashita Date: Fri, 16 Jun 2023 02:12:06 +0900 Subject: [PATCH 07/13] =?UTF-8?q?doc=E3=82=92=E6=9B=B4=E6=96=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/voicevox_core_c_api/src/owners.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/voicevox_core_c_api/src/owners.rs b/crates/voicevox_core_c_api/src/owners.rs index 5318c22f7..2d236be90 100644 --- a/crates/voicevox_core_c_api/src/owners.rs +++ b/crates/voicevox_core_c_api/src/owners.rs @@ -5,6 +5,8 @@ //! //! しかし実装としては`Box`の場合とは異なり、何かしらの情報をRust側で保持し続けなくてはならない。 //! 実態としてはRust側がバッファの所有者(owner)であり続け、C側にはその参照が渡される形になる。 +//! +//! 本モジュールはそのバッファの所有者を提供する。 mod c_string; mod slice; From 28a8c26e4e965e87b142b9d565e0ca9ddf9cc111 Mon Sep 17 00:00:00 2001 From: Ryo Yamashita Date: Fri, 16 Jun 2023 02:38:31 +0900 Subject: [PATCH 08/13] =?UTF-8?q?doc=E3=82=92=E6=9B=B4=E6=96=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/voicevox_core_c_api/src/owners.rs | 3 ++- crates/voicevox_core_c_api/src/owners/c_string.rs | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/voicevox_core_c_api/src/owners.rs b/crates/voicevox_core_c_api/src/owners.rs index 2d236be90..8dd794e56 100644 --- a/crates/voicevox_core_c_api/src/owners.rs +++ b/crates/voicevox_core_c_api/src/owners.rs @@ -6,7 +6,8 @@ //! しかし実装としては`Box`の場合とは異なり、何かしらの情報をRust側で保持し続けなくてはならない。 //! 実態としてはRust側がバッファの所有者(owner)であり続け、C側にはその参照が渡される形になる。 //! -//! 本モジュールはそのバッファの所有者を提供する。 +//! 本モジュールはそのバッファの所有者(owner)を提供する。 +//! 各ownerは実際にRustのオブジェクトを保持し続ける。 mod c_string; mod slice; diff --git a/crates/voicevox_core_c_api/src/owners/c_string.rs b/crates/voicevox_core_c_api/src/owners/c_string.rs index 7244117b5..a38f221d2 100644 --- a/crates/voicevox_core_c_api/src/owners/c_string.rs +++ b/crates/voicevox_core_c_api/src/owners/c_string.rs @@ -6,13 +6,13 @@ use std::{ sync::Mutex, }; +/// Cの世界に貸し出す`CStr`の所有者。 +/// +/// `Mutex`による内部可変性を持ち、すべての操作は共有参照から行うことができる。 pub(crate) static C_STRING_OWNER: CStringOwner = CStringOwner::new(); pub(crate) struct CStringOwner(Mutex); -/// Cの世界に貸し出す`CStr`の所有者。 -/// -/// `Mutex`による内部可変性を持ち、すべての操作は共有参照から行うことができる。 struct CStringOwnerInner { owned_c_strings: BTreeMap>, static_str_addrs: BTreeSet, From 156c2b21610158bef8c09ce732be0e3bb2024dee Mon Sep 17 00:00:00 2001 From: Ryo Yamashita Date: Fri, 16 Jun 2023 02:39:02 +0900 Subject: [PATCH 09/13] =?UTF-8?q?`CStringOwnerInner`=20=E2=86=92=20`Inner`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/voicevox_core_c_api/src/owners/c_string.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/voicevox_core_c_api/src/owners/c_string.rs b/crates/voicevox_core_c_api/src/owners/c_string.rs index a38f221d2..7230eff67 100644 --- a/crates/voicevox_core_c_api/src/owners/c_string.rs +++ b/crates/voicevox_core_c_api/src/owners/c_string.rs @@ -11,16 +11,16 @@ use std::{ /// `Mutex`による内部可変性を持ち、すべての操作は共有参照から行うことができる。 pub(crate) static C_STRING_OWNER: CStringOwner = CStringOwner::new(); -pub(crate) struct CStringOwner(Mutex); +pub(crate) struct CStringOwner(Mutex); -struct CStringOwnerInner { +struct Inner { owned_c_strings: BTreeMap>, static_str_addrs: BTreeSet, } impl CStringOwner { const fn new() -> Self { - Self(Mutex::new(CStringOwnerInner { + Self(Mutex::new(Inner { owned_c_strings: BTreeMap::new(), static_str_addrs: BTreeSet::new(), })) @@ -28,7 +28,7 @@ impl CStringOwner { /// `CString`を所有し、そのポインタを参照としてC API利用者に与える。 pub(crate) fn own_and_lend(&self, s: CString, out: &mut MaybeUninit<*mut c_char>) { - let CStringOwnerInner { + let Inner { owned_c_strings, .. } = &mut *self.0.lock().unwrap(); @@ -46,7 +46,7 @@ impl CStringOwner { /// /// `ptr`が`own_and_lend`で貸し出されたポインタではないとき、パニックする。 pub(crate) fn delete(&self, ptr: *mut c_char) { - let CStringOwnerInner { + let Inner { owned_c_strings, static_str_addrs, .. @@ -72,7 +72,7 @@ impl CStringOwner { /// /// 記憶したポインタは`delete`でのパニックメッセージの切り替えに使われる。 pub(crate) fn memorize_static(&self, s: &'static CStr) -> *const c_char { - let CStringOwnerInner { + let Inner { static_str_addrs, .. } = &mut *self.0.lock().unwrap(); From c1f2a1ffc2b1cbdaff7810fd7f4774a3c508581b Mon Sep 17 00:00:00 2001 From: Ryo Yamashita Date: Fri, 16 Jun 2023 22:32:35 +0900 Subject: [PATCH 10/13] =?UTF-8?q?`delete`=20=E2=86=92=20`drop=5Ffor`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/voicevox_core_c_api/src/lib.rs | 4 ++-- crates/voicevox_core_c_api/src/owners/c_string.rs | 6 +++--- crates/voicevox_core_c_api/src/owners/slice.rs | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/voicevox_core_c_api/src/lib.rs b/crates/voicevox_core_c_api/src/lib.rs index 08c844bef..06d50fed7 100644 --- a/crates/voicevox_core_c_api/src/lib.rs +++ b/crates/voicevox_core_c_api/src/lib.rs @@ -645,7 +645,7 @@ pub unsafe extern "C" fn voicevox_synthesizer_tts( /// @param voicevox_audio_query で確保されたポインタであり、かつ呼び出し側でバッファの変更を行われていないこと #[no_mangle] pub extern "C" fn voicevox_json_free(json: *mut c_char) { - C_STRING_OWNER.delete(json); + C_STRING_OWNER.drop_for(json); } /// wav データのメモリを解放する @@ -655,7 +655,7 @@ pub extern "C" fn voicevox_json_free(json: *mut c_char) { /// @param wav voicevox_tts,voicevox_synthesis で確保されたポインタであり、かつ呼び出し側でバッファの変更を行われていないこと #[no_mangle] pub extern "C" fn voicevox_wav_free(wav: *mut u8) { - U8_SLICE_OWNER.delete(wav); + U8_SLICE_OWNER.drop_for(wav); } /// エラー結果をメッセージに変換する diff --git a/crates/voicevox_core_c_api/src/owners/c_string.rs b/crates/voicevox_core_c_api/src/owners/c_string.rs index 7230eff67..0ff6aa431 100644 --- a/crates/voicevox_core_c_api/src/owners/c_string.rs +++ b/crates/voicevox_core_c_api/src/owners/c_string.rs @@ -45,7 +45,7 @@ impl CStringOwner { /// # Panics /// /// `ptr`が`own_and_lend`で貸し出されたポインタではないとき、パニックする。 - pub(crate) fn delete(&self, ptr: *mut c_char) { + pub(crate) fn drop_for(&self, ptr: *mut c_char) { let Inner { owned_c_strings, static_str_addrs, @@ -95,7 +95,7 @@ mod tests { fn it_denies_unknown_char_ptr() { let owner = CStringOwner::new(); let s = CStr::from_bytes_with_nul(b"\0").unwrap().to_owned(); - owner.delete(s.into_raw()); + owner.drop_for(s.into_raw()); } #[test] @@ -105,7 +105,7 @@ mod tests { fn it_denies_known_static_char_ptr() { let owner = CStringOwner::new(); owner.memorize_static(STATIC); - owner.delete(STATIC.as_ptr() as *mut c_char); + owner.drop_for(STATIC.as_ptr() as *mut c_char); static STATIC: &CStr = unsafe { CStr::from_bytes_with_nul_unchecked(b"\0") }; } diff --git a/crates/voicevox_core_c_api/src/owners/slice.rs b/crates/voicevox_core_c_api/src/owners/slice.rs index 3df0fd7bb..52047a3f0 100644 --- a/crates/voicevox_core_c_api/src/owners/slice.rs +++ b/crates/voicevox_core_c_api/src/owners/slice.rs @@ -41,7 +41,7 @@ impl SliceOwner { /// # Panics /// /// `ptr`が`own_and_lend`で貸し出されたポインタではないとき、パニックする。 - pub(crate) fn delete(&self, ptr: *mut T) { + pub(crate) fn drop_for(&self, ptr: *mut T) { let mut slices = self.slices.lock().unwrap(); slices.remove(&(ptr as usize)).expect( @@ -81,7 +81,7 @@ mod tests { (ptr.assume_init(), len.assume_init()) }; assert_eq!(expected_len, len); - owner.delete(ptr); + owner.drop_for(ptr); } fn vec(initial_cap: usize, elems: &[T]) -> Vec { @@ -98,6 +98,6 @@ mod tests { fn it_denies_unknown_ptr() { let owner = SliceOwner::::new(); let x = 42; - owner.delete(&x as *const i32 as *mut i32); + owner.drop_for(&x as *const i32 as *mut i32); } } From 6573067ef612fa2afa8aab2d3a48362a344984de Mon Sep 17 00:00:00 2001 From: Ryo Yamashita Date: Sat, 17 Jun 2023 16:09:43 +0900 Subject: [PATCH 11/13] =?UTF-8?q?`CStringOwner`=20=E2=86=92=20`CStringDrop?= =?UTF-8?q?Checker`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/voicevox_core_c_api/src/drop_check.rs | 126 ++++++++++++++++++ crates/voicevox_core_c_api/src/lib.rs | 40 +++--- crates/voicevox_core_c_api/src/owners.rs | 15 --- .../src/owners/c_string.rs | 112 ---------------- .../src/{owners/slice.rs => slice_owner.rs} | 12 +- 5 files changed, 162 insertions(+), 143 deletions(-) create mode 100644 crates/voicevox_core_c_api/src/drop_check.rs delete mode 100644 crates/voicevox_core_c_api/src/owners.rs delete mode 100644 crates/voicevox_core_c_api/src/owners/c_string.rs rename crates/voicevox_core_c_api/src/{owners/slice.rs => slice_owner.rs} (81%) diff --git a/crates/voicevox_core_c_api/src/drop_check.rs b/crates/voicevox_core_c_api/src/drop_check.rs new file mode 100644 index 000000000..596f54171 --- /dev/null +++ b/crates/voicevox_core_c_api/src/drop_check.rs @@ -0,0 +1,126 @@ +use std::{ + collections::BTreeSet, + ffi::{c_char, CStr, CString}, + sync::Mutex, +}; + +/// `*mut c_char`を`CString`としてdropしていいかチェックする。 +/// +/// `Mutex`による内部可変性を持ち、すべての操作は共有参照から行うことができる。 +/// +/// # Motivation +/// +/// `CString`は`Box`と同様Cの世界でもポインタ一つで実体を表すことができるため、こちら側 +/// で管理すべきものは本来無い。しかしながら本クレートが提供するAPIには「解放不要」な文字列を返すも +/// のが含まれている。ユーザーが誤ってそのような文字列を解放するのは未定義動作 (undefined behavior) +/// であるため、綺麗にSEGVするとも限らない。`once_cell::sync::Lazy`由来の文字列の場合、最悪解放が成 +/// 功してしまう。 +/// +/// この構造体はCの世界から帰ってきた`*mut c_char`を`CString`としてdropする際、それが本当にこちら側 +/// が送り出した`CString`かどうかをチェックする。 +/// +/// Cの世界に`CString`を送り出す前に`whitelist`を通し、戻って来た`*mut c_char`を`CString`にしてdrop +/// する前に`check`に通す。 +pub(crate) static C_STRING_DROP_CHECKER: CStringDropChecker = CStringDropChecker::new(); + +pub(crate) struct CStringDropChecker(Mutex); + +struct Inner { + owned_str_addrs: BTreeSet, + static_str_addrs: BTreeSet, +} + +impl CStringDropChecker { + const fn new() -> Self { + Self(Mutex::new(Inner { + owned_str_addrs: BTreeSet::new(), + static_str_addrs: BTreeSet::new(), + })) + } + + /// `CString`をホワイトリストに追加する。 + /// + /// Cの世界に`CString`を送り出す前にこの関数を挟む。 + pub(crate) fn whitelist(&self, s: CString) -> CString { + let Inner { + owned_str_addrs, .. + } = &mut *self.0.lock().unwrap(); + + let duplicated = !owned_str_addrs.insert(s.as_ptr() as usize); + assert!(!duplicated, "duplicated"); + s + } + + /// `&'static CStr`をブラックリストに追加する。 + /// + /// Cの世界に`Lazy`由来の`&'static CStr`を送り出す前にこの関数を挟む。 + /// + /// ホワイトリストとブラックリストは重複しないと考えてよく、ブラックリストはエラーメセージの制御 + /// のためのみに使われる。 + pub(crate) fn blacklist(&self, s: &'static CStr) -> &'static CStr { + let Inner { + static_str_addrs, .. + } = &mut *self.0.lock().unwrap(); + + static_str_addrs.insert(s.as_ptr() as usize); + s + } + + /// `*mut c_char`が`whitelist`を通ったものかどうかチェックする。 + /// + /// # Panics + /// + /// `ptr`が`Self::whitelist`を経由したものではないならパニックする。 + pub(crate) fn check(&self, ptr: *mut c_char) -> *mut c_char { + let Inner { + owned_str_addrs, + static_str_addrs, + .. + } = &mut *self.0.lock().unwrap(); + + let addr = ptr as usize; + if !owned_str_addrs.remove(&addr) { + if static_str_addrs.contains(&addr) { + panic!( + "解放しようとしたポインタはvoicevox_core管理下のものですが、\ + voicevox_coreがアンロードされるまで永続する文字列に対するものです。\ + 解放することはできません", + ) + } + panic!( + "解放しようとしたポインタはvoicevox_coreの管理下にありません。\ + 誤ったポインタであるか、二重解放になっていることが考えられます", + ); + } + ptr + } +} + +#[cfg(test)] +mod tests { + use std::ffi::{c_char, CStr}; + + use super::CStringDropChecker; + + #[test] + #[should_panic( + expected = "解放しようとしたポインタはvoicevox_coreの管理下にありません。誤ったポインタであるか、二重解放になっていることが考えられます" + )] + fn it_denies_unknown_char_ptr() { + let checker = CStringDropChecker::new(); + let s = CStr::from_bytes_with_nul(b"\0").unwrap().to_owned(); + checker.check(s.into_raw()); + } + + #[test] + #[should_panic( + expected = "解放しようとしたポインタはvoicevox_core管理下のものですが、voicevox_coreがアンロードされるまで永続する文字列に対するものです。解放することはできません" + )] + fn it_denies_known_static_char_ptr() { + let checker = CStringDropChecker::new(); + checker.blacklist(STATIC); + checker.check(STATIC.as_ptr() as *mut c_char); + + static STATIC: &CStr = unsafe { CStr::from_bytes_with_nul_unchecked(b"\0") }; + } +} diff --git a/crates/voicevox_core_c_api/src/lib.rs b/crates/voicevox_core_c_api/src/lib.rs index 06d50fed7..1036b729c 100644 --- a/crates/voicevox_core_c_api/src/lib.rs +++ b/crates/voicevox_core_c_api/src/lib.rs @@ -1,10 +1,12 @@ mod c_impls; /// cbindgen:ignore mod compatible_engine; +mod drop_check; mod helpers; -mod owners; +mod slice_owner; +use self::drop_check::C_STRING_DROP_CHECKER; use self::helpers::*; -use self::owners::{C_STRING_OWNER, U8_SLICE_OWNER}; +use self::slice_owner::U8_SLICE_OWNER; use chrono::SecondsFormat; use const_default::ConstDefault; use derive_getters::Getters; @@ -355,7 +357,11 @@ pub extern "C" fn voicevox_create_supported_devices_json( into_result_code_with_error((|| { let supported_devices = CString::new(SupportedDevices::create()?.to_json().to_string()).unwrap(); - C_STRING_OWNER.own_and_lend(supported_devices, output_supported_devices_json); + output_supported_devices_json.write( + C_STRING_DROP_CHECKER + .whitelist(supported_devices) + .into_raw(), + ); Ok(()) })()) } @@ -400,7 +406,7 @@ pub unsafe extern "C" fn voicevox_synthesizer_audio_query( ))?; let audio_query = CString::new(audio_query_model_to_json(&audio_query)) .expect("should not contain '\\0'"); - C_STRING_OWNER.own_and_lend(audio_query, output_audio_query_json); + output_audio_query_json.write(C_STRING_DROP_CHECKER.whitelist(audio_query).into_raw()); Ok(()) })()) } @@ -443,7 +449,8 @@ pub unsafe extern "C" fn voicevox_synthesizer_create_accent_phrases( ))?; let accent_phrases = CString::new(accent_phrases_to_json(&accent_phrases)) .expect("should not contain '\\0'"); - C_STRING_OWNER.own_and_lend(accent_phrases, output_accent_phrases_json); + output_accent_phrases_json + .write(C_STRING_DROP_CHECKER.whitelist(accent_phrases).into_raw()); Ok(()) })()) } @@ -475,7 +482,8 @@ pub unsafe extern "C" fn voicevox_synthesizer_replace_mora_data( )?; let accent_phrases = CString::new(accent_phrases_to_json(&accent_phrases)) .expect("should not contain '\\0'"); - C_STRING_OWNER.own_and_lend(accent_phrases, output_accent_phrases_json); + output_accent_phrases_json + .write(C_STRING_DROP_CHECKER.whitelist(accent_phrases).into_raw()); Ok(()) })()) } @@ -507,7 +515,8 @@ pub unsafe extern "C" fn voicevox_synthesizer_replace_phoneme_length( )?; let accent_phrases = CString::new(accent_phrases_to_json(&accent_phrases)) .expect("should not contain '\\0'"); - C_STRING_OWNER.own_and_lend(accent_phrases, output_accent_phrases_json); + output_accent_phrases_json + .write(C_STRING_DROP_CHECKER.whitelist(accent_phrases).into_raw()); Ok(()) })()) } @@ -539,7 +548,8 @@ pub unsafe extern "C" fn voicevox_synthesizer_replace_mora_pitch( )?; let accent_phrases = CString::new(accent_phrases_to_json(&accent_phrases)) .expect("should not contain '\\0'"); - C_STRING_OWNER.own_and_lend(accent_phrases, output_accent_phrases_json); + output_accent_phrases_json + .write(C_STRING_DROP_CHECKER.whitelist(accent_phrases).into_raw()); Ok(()) })()) } @@ -644,8 +654,8 @@ pub unsafe extern "C" fn voicevox_synthesizer_tts( /// # Safety /// @param voicevox_audio_query で確保されたポインタであり、かつ呼び出し側でバッファの変更を行われていないこと #[no_mangle] -pub extern "C" fn voicevox_json_free(json: *mut c_char) { - C_STRING_OWNER.drop_for(json); +pub unsafe extern "C" fn voicevox_json_free(json: *mut c_char) { + drop(CString::from_raw(C_STRING_DROP_CHECKER.check(json))); } /// wav データのメモリを解放する @@ -665,12 +675,12 @@ pub extern "C" fn voicevox_wav_free(wav: *mut u8) { pub extern "C" fn voicevox_error_result_to_message( result_code: VoicevoxResultCode, ) -> *const c_char { - C_STRING_OWNER.memorize_static( - CStr::from_bytes_with_nul( - voicevox_core::result_code::error_result_to_message(result_code).as_ref(), - ) - .unwrap(), + let message = CStr::from_bytes_with_nul( + voicevox_core::result_code::error_result_to_message(result_code).as_ref(), ) + .expect("`error_result_to_message` should always return NUL-terminated string"); + + C_STRING_DROP_CHECKER.blacklist(message).as_ptr() } #[cfg(test)] diff --git a/crates/voicevox_core_c_api/src/owners.rs b/crates/voicevox_core_c_api/src/owners.rs deleted file mode 100644 index 8dd794e56..000000000 --- a/crates/voicevox_core_c_api/src/owners.rs +++ /dev/null @@ -1,15 +0,0 @@ -//! `Box<[u8]>`や`CString`といったバッファの所有者(owner)。 -//! -//! 本クレートが提供するAPIとして、バイト列/文字列の生成(create)とその解放(free)がある。 -//! APIとしては"生成"時に`Box<[u8]>`/`CString`のownershipがC側に渡され、"解放"時にはそのownershipがRust側に返されるといった形となる。 -//! -//! しかし実装としては`Box`の場合とは異なり、何かしらの情報をRust側で保持し続けなくてはならない。 -//! 実態としてはRust側がバッファの所有者(owner)であり続け、C側にはその参照が渡される形になる。 -//! -//! 本モジュールはそのバッファの所有者(owner)を提供する。 -//! 各ownerは実際にRustのオブジェクトを保持し続ける。 - -mod c_string; -mod slice; - -pub(crate) use self::{c_string::C_STRING_OWNER, slice::U8_SLICE_OWNER}; diff --git a/crates/voicevox_core_c_api/src/owners/c_string.rs b/crates/voicevox_core_c_api/src/owners/c_string.rs deleted file mode 100644 index 0ff6aa431..000000000 --- a/crates/voicevox_core_c_api/src/owners/c_string.rs +++ /dev/null @@ -1,112 +0,0 @@ -use std::{ - cell::UnsafeCell, - collections::{BTreeMap, BTreeSet}, - ffi::{c_char, CStr, CString}, - mem::MaybeUninit, - sync::Mutex, -}; - -/// Cの世界に貸し出す`CStr`の所有者。 -/// -/// `Mutex`による内部可変性を持ち、すべての操作は共有参照から行うことができる。 -pub(crate) static C_STRING_OWNER: CStringOwner = CStringOwner::new(); - -pub(crate) struct CStringOwner(Mutex); - -struct Inner { - owned_c_strings: BTreeMap>, - static_str_addrs: BTreeSet, -} - -impl CStringOwner { - const fn new() -> Self { - Self(Mutex::new(Inner { - owned_c_strings: BTreeMap::new(), - static_str_addrs: BTreeSet::new(), - })) - } - - /// `CString`を所有し、そのポインタを参照としてC API利用者に与える。 - pub(crate) fn own_and_lend(&self, s: CString, out: &mut MaybeUninit<*mut c_char>) { - let Inner { - owned_c_strings, .. - } = &mut *self.0.lock().unwrap(); - - let ptr = s.as_ptr() as *mut c_char; - - let duplicated = owned_c_strings.insert(ptr as usize, s.into()).is_some(); - assert!(!duplicated, "duplicated"); - - out.write(ptr); - } - - /// `own_and_lend`でC API利用者に貸し出したポインタに対応する`CString`をデストラクトする。 - /// - /// # Panics - /// - /// `ptr`が`own_and_lend`で貸し出されたポインタではないとき、パニックする。 - pub(crate) fn drop_for(&self, ptr: *mut c_char) { - let Inner { - owned_c_strings, - static_str_addrs, - .. - } = &mut *self.0.lock().unwrap(); - - let addr = ptr as usize; - if owned_c_strings.remove(&addr).is_none() { - if static_str_addrs.contains(&addr) { - panic!( - "解放しようとしたポインタはvoicevox_core管理下のものですが、\ - voicevox_coreがアンロードされるまで永続する文字列に対するものです。\ - 解放することはできません", - ) - } - panic!( - "解放しようとしたポインタはvoicevox_coreの管理下にありません。\ - 誤ったポインタであるか、二重解放になっていることが考えられます", - ); - } - } - - /// `&'static CStr`のアドレスを記憶する。 - /// - /// 記憶したポインタは`delete`でのパニックメッセージの切り替えに使われる。 - pub(crate) fn memorize_static(&self, s: &'static CStr) -> *const c_char { - let Inner { - static_str_addrs, .. - } = &mut *self.0.lock().unwrap(); - - let ptr = s.as_ptr(); - static_str_addrs.insert(ptr as usize); - ptr - } -} - -#[cfg(test)] -mod tests { - use std::ffi::{c_char, CStr}; - - use super::CStringOwner; - - #[test] - #[should_panic( - expected = "解放しようとしたポインタはvoicevox_coreの管理下にありません。誤ったポインタであるか、二重解放になっていることが考えられます" - )] - fn it_denies_unknown_char_ptr() { - let owner = CStringOwner::new(); - let s = CStr::from_bytes_with_nul(b"\0").unwrap().to_owned(); - owner.drop_for(s.into_raw()); - } - - #[test] - #[should_panic( - expected = "解放しようとしたポインタはvoicevox_core管理下のものですが、voicevox_coreがアンロードされるまで永続する文字列に対するものです。解放することはできません" - )] - fn it_denies_known_static_char_ptr() { - let owner = CStringOwner::new(); - owner.memorize_static(STATIC); - owner.drop_for(STATIC.as_ptr() as *mut c_char); - - static STATIC: &CStr = unsafe { CStr::from_bytes_with_nul_unchecked(b"\0") }; - } -} diff --git a/crates/voicevox_core_c_api/src/owners/slice.rs b/crates/voicevox_core_c_api/src/slice_owner.rs similarity index 81% rename from crates/voicevox_core_c_api/src/owners/slice.rs rename to crates/voicevox_core_c_api/src/slice_owner.rs index 52047a3f0..8998be7be 100644 --- a/crates/voicevox_core_c_api/src/owners/slice.rs +++ b/crates/voicevox_core_c_api/src/slice_owner.rs @@ -1,8 +1,18 @@ use std::{cell::UnsafeCell, collections::BTreeMap, mem::MaybeUninit, sync::Mutex}; -/// Cの世界に貸し出す`[u8]`の所有者。 +/// Cの世界に貸し出す`[u8]`の所有者(owner)。 /// /// `Mutex`による内部可変性を持ち、すべての操作は共有参照から行うことができる。 +/// +/// # Motivation +/// +/// 本クレートが提供するAPIとして、バイト列の生成(create)とその解放(free)がある。APIとしては"生成 +/// "時に`Box<[u8]>`のownershipがC側に渡され、"解放"時にはそのownershipがRust側に返されるといった形 +/// となる。 +/// +/// しかし実装としては`Box`の場合とは異なり、何かしらの情報をRust側で保持し続けなくては +/// ならない。実態としてはRust側がバッファの所有者(owner)であり続け、C側にはその参照が渡される形にな +/// る。この構造体はその"所有者"であり、実際にRustのオブジェクトを保持し続ける。 pub(crate) static U8_SLICE_OWNER: SliceOwner = SliceOwner::new(); pub(crate) struct SliceOwner { From 5e361ba4cf108982fd0072a0482716cfbc91ba2e Mon Sep 17 00:00:00 2001 From: Hiroshiba Date: Sun, 18 Jun 2023 21:17:33 +0900 Subject: [PATCH 12/13] Update crates/voicevox_core_c_api/src/lib.rs Co-authored-by: Ryo Yamashita --- crates/voicevox_core_c_api/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/voicevox_core_c_api/src/lib.rs b/crates/voicevox_core_c_api/src/lib.rs index 1036b729c..334cc03f4 100644 --- a/crates/voicevox_core_c_api/src/lib.rs +++ b/crates/voicevox_core_c_api/src/lib.rs @@ -678,7 +678,7 @@ pub extern "C" fn voicevox_error_result_to_message( let message = CStr::from_bytes_with_nul( voicevox_core::result_code::error_result_to_message(result_code).as_ref(), ) - .expect("`error_result_to_message` should always return NUL-terminated string"); + .expect("`error_result_to_message`が返す文字列はヌル終端であるはずである"); C_STRING_DROP_CHECKER.blacklist(message).as_ptr() } From f14844685cd8357cb40035476fbc5c22fc0ca974 Mon Sep 17 00:00:00 2001 From: Ryo Yamashita Date: Mon, 19 Jun 2023 02:37:12 +0900 Subject: [PATCH 13/13] =?UTF-8?q?`CStringDropChecker`=E3=81=AEdoc=E3=82=92?= =?UTF-8?q?=E6=9B=B4=E6=96=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Hiroshiba --- crates/voicevox_core_c_api/src/drop_check.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/voicevox_core_c_api/src/drop_check.rs b/crates/voicevox_core_c_api/src/drop_check.rs index 596f54171..546739dbd 100644 --- a/crates/voicevox_core_c_api/src/drop_check.rs +++ b/crates/voicevox_core_c_api/src/drop_check.rs @@ -4,7 +4,7 @@ use std::{ sync::Mutex, }; -/// `*mut c_char`を`CString`としてdropしていいかチェックする。 +/// dropして良い`*mut c_char`を把握し、チェックする。 /// /// `Mutex`による内部可変性を持ち、すべての操作は共有参照から行うことができる。 ///