Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix up #849 #862

Merged
merged 1 commit into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ async_zip = "=0.0.16"
bindgen = "0.69.4"
binstall-tar = "0.4.42"
blocking = "1.6.1"
boxcar = "0.2.6"
bytes = "1.7.2"
camino = "1.1.9"
cargo_metadata = "0.18.1"
Expand Down
1 change: 1 addition & 0 deletions crates/voicevox_core_c_api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ link-onnxruntime = ["voicevox_core/link-onnxruntime"]
[dependencies]
anstream = { workspace = true, default-features = false, features = ["auto"] }
anstyle-query.workspace = true
boxcar.workspace = true
camino.workspace = true
chrono = { workspace = true, default-features = false, features = ["clock"] }
colorchoice.workspace = true
Expand Down
17 changes: 12 additions & 5 deletions crates/voicevox_core_c_api/src/c_impls.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::{
collections::HashMap,
collections::{HashMap, HashSet},
ffi::CString,
num::NonZero,
path::Path,
ptr::NonNull,
sync::{Arc, LazyLock},
Expand Down Expand Up @@ -145,17 +146,23 @@ fn metas_to_json(metas: &[SpeakerMeta]) -> CString {
impl CApiObject for H {
type RustApiObject = B;

fn heads() -> &'static std::sync::Mutex<Vec<Self>> {
static HEADS: std::sync::Mutex<Vec<H>> = std::sync::Mutex::new(vec![]);
fn known_addrs() -> &'static std::sync::Mutex<HashSet<NonZero<usize>>> {
static KNOWN_ADDRS: LazyLock<std::sync::Mutex<HashSet<NonZero<usize>>>> =
LazyLock::new(Default::default);
&KNOWN_ADDRS
}

fn heads() -> &'static boxcar::Vec<Self> {
static HEADS: boxcar::Vec<H> = boxcar::Vec::new();
&HEADS
}

fn bodies() -> &'static std::sync::Mutex<
HashMap<usize, Arc<parking_lot::RwLock<Option<Self::RustApiObject>>>>,
HashMap<NonZero<usize>, Arc<parking_lot::RwLock<Option<Self::RustApiObject>>>>,
> {
#[expect(clippy::type_complexity, reason = "`CApiObject::bodies`と同様")]
static BODIES: LazyLock<
std::sync::Mutex<HashMap<usize, Arc<parking_lot::RwLock<Option<B>>>>>,
std::sync::Mutex<HashMap<NonZero<usize>, Arc<parking_lot::RwLock<Option<B>>>>>,
> = LazyLock::new(Default::default);

&BODIES
Expand Down
75 changes: 48 additions & 27 deletions crates/voicevox_core_c_api/src/object.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use std::{
any,
collections::HashMap,
collections::{HashMap, HashSet},
fmt::{Debug, Display},
mem,
num::NonZero,
ops::{Deref, DerefMut},
ptr::NonNull,
sync::Arc,
Expand All @@ -11,6 +12,9 @@ use std::{
use easy_ext::ext;
use tracing::warn;

// FIXME: 次のような状況に備え、`new`をいっぱい行うテストを書く
// https://github.com/VOICEVOX/voicevox_core/pull/849#discussion_r1814221605

/// プロセスの終わりまでデストラクトされない、ユーザーにオブジェクトとして貸し出す1-bit長の構造体。
///
/// インスタンスは次のような形。
Expand All @@ -31,16 +35,18 @@ use tracing::warn;
pub(crate) trait CApiObject: Default + Debug + 'static {
type RustApiObject: 'static;

// 書き込み操作としては`push`のみ
fn heads() -> &'static std::sync::Mutex<Vec<Self>>;
// 行う可変操作は`insert`のみ
fn known_addrs() -> &'static std::sync::Mutex<HashSet<NonZero<usize>>>;

fn heads() -> &'static boxcar::Vec<Self>;

#[expect(
clippy::type_complexity,
reason = "型を分離するとかえって可読性を失う。その代わりコメントを入れている"
)]
fn bodies() -> &'static std::sync::Mutex<
HashMap<
usize, // `heads`の要素へのポインタのアドレス
NonZero<usize>, // `heads`の要素へのポインタのアドレス
Arc<
parking_lot::RwLock<
Option<Self::RustApiObject>, // `RwLock`をdropする直前まで`Some`
Expand All @@ -53,71 +59,79 @@ pub(crate) trait CApiObject: Default + Debug + 'static {
assert!(mem::size_of::<Self>() > 0);

let this = {
let mut heads = Self::lock_heads();
heads.push(Default::default());
NonNull::from(heads.last().expect("just pushed"))
let i = Self::heads().push(Default::default());
NonNull::from(&Self::heads()[i])
};
Self::lock_known_addrs().insert(this.addr_without_provenance());
let body = parking_lot::RwLock::new(body.into()).into();
Self::lock_bodies().insert(this.as_ptr() as _, body);
Self::lock_bodies().insert(this.addr_without_provenance(), body);
this
}
}

#[ext(CApiObjectPtrExt)]
impl<T: CApiObject> *const T {
// ユーザーから渡されたポインタである`self`は、次のうちどれかに分類される。
//
// 1. `known_addrs`に含まれない ⇨ 知らないどこかのダングリングポインタか何か。あるいはnull
// 2. `known_addrs`に含まれるが、`bodies`に含まれない → "delete"済み
// 3. `known_addrs`も`bodies`にも含まれる → 1.でも2.でもなく、有効

/// # Panics
///
/// 同じ対象に対して`drop_body`を呼んでいるとパニックする。
pub(crate) fn body(self) -> impl Deref<Target = T::RustApiObject> {
self.validate();
let this = self.validate();

let body = T::lock_bodies()
.get(&(self as _))
.unwrap_or_else(|| self.panic_for_deleted())
.get(&this.addr_without_provenance())
.unwrap_or_else(|| this.panic_for_deleted())
.read_arc();

voicevox_core::__internal::interop::raii::try_map_guard(body, |body| {
body.as_ref().ok_or(())
})
.unwrap_or_else(|()| self.panic_for_deleted())
.unwrap_or_else(|()| this.panic_for_deleted())
}

/// # Panics
///
/// 同じ対象に対してこの関数を二度呼ぶとパニックする。
pub(crate) fn drop_body(self) {
self.validate();
let this = self.validate();

let body = T::lock_bodies()
.remove(&(self as _))
.unwrap_or_else(|| self.panic_for_deleted());
.remove(&this.addr_without_provenance())
.unwrap_or_else(|| this.panic_for_deleted());

drop(
body.try_write_arc()
.unwrap_or_else(|| {
warn!(
"{this} is still in use. Waiting before closing",
this = self.display(),
this = this.display(),
);
body.write_arc()
})
.take()
.unwrap_or_else(|| self.panic_for_deleted()),
.unwrap_or_else(|| this.panic_for_deleted()),
);
}
}

#[ext]
impl<T: CApiObject> *const T {
fn validate(self) {
if self.is_null() {
panic!("the argument must not be null");
}
if !T::lock_heads().as_ptr_range().contains(&self) {
fn validate(self) -> NonNull<T> {
let this = NonNull::new(self as *mut T).expect("the argument must not be null");
if !T::lock_known_addrs().contains(&this.addr_without_provenance()) {
panic!("{self:018p} does not seem to be valid object");
}
this
}
}

#[ext]
impl<T: CApiObject> NonNull<T> {
fn display(self) -> impl Display {
let type_name = any::type_name::<T>()
.split("::")
Expand All @@ -131,15 +145,22 @@ impl<T: CApiObject> *const T {
}
}

#[ext]
impl<T> NonNull<T> {
fn addr_without_provenance(self) -> NonZero<usize> {
NonZero::new(self.as_ptr() as _).expect("this is from `NonNull`")
}
}

#[ext]
impl<T: CApiObject> T {
fn lock_heads() -> impl DerefMut<Target = Vec<Self>> {
Self::heads().lock().unwrap_or_else(|e| panic!("{e}"))
fn lock_known_addrs() -> impl DerefMut<Target = HashSet<NonZero<usize>>> {
Self::known_addrs().lock().unwrap_or_else(|e| panic!("{e}"))
}

fn lock_bodies(
) -> impl DerefMut<Target = HashMap<usize, Arc<parking_lot::RwLock<Option<Self::RustApiObject>>>>>
{
fn lock_bodies() -> impl DerefMut<
Target = HashMap<NonZero<usize>, Arc<parking_lot::RwLock<Option<Self::RustApiObject>>>>,
> {
Self::bodies().lock().unwrap_or_else(|e| panic!("{e}"))
}
}
Loading