diff --git a/Cargo.lock b/Cargo.lock index 26e6fb9e9..7eff522e6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4491,7 +4491,6 @@ dependencies = [ "futures-lite", "log", "once_cell", - "ouroboros", "pyo3", "pyo3-asyncio", "pyo3-log", diff --git a/crates/voicevox_core/src/__internal/interop.rs b/crates/voicevox_core/src/__internal/interop.rs index 677f5515a..a218730cd 100644 --- a/crates/voicevox_core/src/__internal/interop.rs +++ b/crates/voicevox_core/src/__internal/interop.rs @@ -1,3 +1,5 @@ +pub mod raii; + pub use crate::{ metas::merge as merge_metas, synthesizer::blocking::PerformInference, voice_model::blocking::IdRef, diff --git a/crates/voicevox_core/src/__internal/interop/raii.rs b/crates/voicevox_core/src/__internal/interop/raii.rs new file mode 100644 index 000000000..048feb005 --- /dev/null +++ b/crates/voicevox_core/src/__internal/interop/raii.rs @@ -0,0 +1,43 @@ +use std::{marker::PhantomData, ops::Deref}; + +use ouroboros::self_referencing; + +pub enum MaybeClosed { + Open(T), + Closed, +} + +// [`mapped_lock_guards`]のようなことをやるためのユーティリティ。 +// +// [`mapped_lock_guards`]: https://github.com/rust-lang/rust/issues/117108 +pub fn try_map_guard<'a, G, F, T, E>(guard: G, f: F) -> Result + 'a, E> +where + G: 'a, + F: FnOnce(&G) -> Result<&T, E>, + T: 'static, +{ + return MappedLockTryBuilder { + guard, + content_builder: f, + marker: PhantomData, + } + .try_build(); + + #[self_referencing] + struct MappedLock<'a, G: 'a, T: 'static> { + guard: G, + + #[borrows(guard)] + content: &'this T, + + marker: PhantomData<&'a ()>, + } + + impl<'a, G: 'a, T: 'static> Deref for MappedLock<'a, G, T> { + type Target = T; + + fn deref(&self) -> &Self::Target { + self.borrow_content() + } + } +} diff --git a/crates/voicevox_core_java_api/lib/src/main/java/jp/hiroshiba/voicevoxcore/Synthesizer.java b/crates/voicevox_core_java_api/lib/src/main/java/jp/hiroshiba/voicevoxcore/Synthesizer.java index 8ac62b9a5..c59f8ca1e 100644 --- a/crates/voicevox_core_java_api/lib/src/main/java/jp/hiroshiba/voicevoxcore/Synthesizer.java +++ b/crates/voicevox_core_java_api/lib/src/main/java/jp/hiroshiba/voicevoxcore/Synthesizer.java @@ -72,9 +72,7 @@ public VoiceModelFile.SpeakerMeta[] metas() { * @throws InvalidModelDataException 無効なモデルデータの場合。 */ public void loadVoiceModel(VoiceModelFile voiceModel) throws InvalidModelDataException { - synchronized (voiceModel) { - rsLoadVoiceModel(voiceModel.opened()); - } + rsLoadVoiceModel(voiceModel); } /** @@ -287,8 +285,7 @@ public TtsConfigurator tts(String text, int styleId) { @Nonnull private native String rsGetMetasJson(); - private native void rsLoadVoiceModel(VoiceModelFile.Opened voiceModel) - throws InvalidModelDataException; + private native void rsLoadVoiceModel(VoiceModelFile voiceModel) throws InvalidModelDataException; private native void rsUnloadVoiceModel(UUID voiceModelId); diff --git a/crates/voicevox_core_java_api/lib/src/main/java/jp/hiroshiba/voicevoxcore/VoiceModelFile.java b/crates/voicevox_core_java_api/lib/src/main/java/jp/hiroshiba/voicevoxcore/VoiceModelFile.java index 599d10e21..1d2a13baa 100644 --- a/crates/voicevox_core_java_api/lib/src/main/java/jp/hiroshiba/voicevoxcore/VoiceModelFile.java +++ b/crates/voicevox_core_java_api/lib/src/main/java/jp/hiroshiba/voicevoxcore/VoiceModelFile.java @@ -6,23 +6,22 @@ import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; import java.io.Closeable; -import java.util.Optional; import java.util.UUID; /** 音声モデルファイル。 */ public class VoiceModelFile extends Dll implements Closeable { + private long handle; + /** ID。 */ @Nonnull public final UUID id; /** メタ情報。 */ @Nonnull public final SpeakerMeta[] metas; - @Nullable private Opened inner; - public VoiceModelFile(String modelPath) { - inner = new Opened(modelPath); - id = inner.rsGetId(); - String metasJson = inner.rsGetMetasJson(); + rsOpen(modelPath); + id = rsGetId(); + String metasJson = rsGetMetasJson(); Gson gson = new Gson(); SpeakerMeta[] rawMetas = gson.fromJson(metasJson, SpeakerMeta[].class); if (rawMetas == null) { @@ -31,48 +30,28 @@ public VoiceModelFile(String modelPath) { metas = rawMetas; } - // `synchronized`は`Synthesizer`側でやる - Opened opened() { - if (inner == null) { - throw new IllegalStateException("this `VoiceModelFile` is closed"); - } - return inner; + @Override + public void close() { + rsClose(); } @Override - public synchronized void close() { - Optional inner = Optional.ofNullable(this.inner); - this.inner = null; - if (inner.isPresent()) { - inner.get().rsDrop(); - } + protected void finalize() throws Throwable { + rsDrop(); + super.finalize(); } - static class Opened { - private long handle; - - private Opened(String modelPath) { - rsOpen(modelPath); - } - - @Override - protected void finalize() throws Throwable { - if (handle != 0) { - rsDrop(); - } - super.finalize(); - } + private native void rsOpen(String modelPath); - private native void rsOpen(String modelPath); + @Nonnull + private native UUID rsGetId(); - @Nonnull - private native UUID rsGetId(); + @Nonnull + private native String rsGetMetasJson(); - @Nonnull - private native String rsGetMetasJson(); + private native void rsClose(); - private native void rsDrop(); - } + private native void rsDrop(); /** 話者（speaker）のメタ情報。 */ public static class SpeakerMeta { diff --git a/crates/voicevox_core_java_api/src/common.rs b/crates/voicevox_core_java_api/src/common.rs index 1b45dd44d..3c2598f33 100644 --- a/crates/voicevox_core_java_api/src/common.rs +++ b/crates/voicevox_core_java_api/src/common.rs @@ -1,4 +1,4 @@ -use std::{error::Error as _, iter}; +use std::{error::Error as _, iter, mem, ops::Deref}; use derive_more::From; use easy_ext::ext; @@ -6,7 +6,9 @@ use jni::{ objects::{JObject, JThrowable}, JNIEnv, }; +use tracing::{debug, warn}; use uuid::Uuid; +use voicevox_core::__internal::interop::raii::MaybeClosed; #[macro_export] macro_rules! object { @@ -154,6 +156,9 @@ where env.throw_new("java/lang/IllegalArgumentException", error.to_string()) ) } + JavaApiError::IllegalState(msg) => { + or_panic!(env.throw_new("java/lang/IllegalStateException", msg)) + } }; } fallback @@ -161,6 +166,8 @@ where } } +type JavaApiResult = Result; + #[derive(From, Debug)] pub(crate) enum JavaApiError { #[from] @@ -173,6 +180,69 @@ pub(crate) enum JavaApiError { Uuid(uuid::Error), DeJson(serde_json::Error), + + IllegalState(String), +} + +pub(crate) struct Closable(std::sync::RwLock>); + +impl Closable { + pub(crate) fn new(content: T) -> Self { + Self(MaybeClosed::Open(content).into()) + } + + pub(crate) fn read(&self) -> JavaApiResult + '_> { + let lock = self.0.try_read().map_err(|e| match e { + std::sync::TryLockError::Poisoned(e) => panic!("{e}"), + std::sync::TryLockError::WouldBlock => { + JavaApiError::IllegalState(format!("The `{}` is being closed", T::JAVA_CLASS_IDENT)) + } + })?; + + voicevox_core::__internal::interop::raii::try_map_guard(lock, |lock| match &**lock { + MaybeClosed::Open(content) => Ok(content), + MaybeClosed::Closed => Err(JavaApiError::IllegalState(format!( + "The `{}` is closed", + T::JAVA_CLASS_IDENT, + ))), + }) + } + + pub(crate) fn close(&self) { + let lock = &mut *match self.0.try_write() { + Ok(lock) => lock, + Err(std::sync::TryLockError::Poisoned(e)) => panic!("{e}"), + Err(std::sync::TryLockError::WouldBlock) => { + self.0.write().unwrap_or_else(|e| panic!("{e}")) + } + }; + + if matches!(*lock, MaybeClosed::Open(_)) { + debug!("Closing a `{}`", T::JAVA_CLASS_IDENT); + } + drop(mem::replace(lock, MaybeClosed::Closed)); + } +} + +impl Drop for Closable { + fn drop(&mut self) { + let content = mem::replace( + &mut *self.0.write().unwrap_or_else(|e| panic!("{e}")), + MaybeClosed::Closed, + ); + if let MaybeClosed::Open(content) = content { + warn!( + "デストラクタにより`{}`のクローズを行います。通常は、可能な限り`close`でクローズす\ + るようにして下さい", + T::JAVA_CLASS_IDENT, + ); + drop(content); + } + } +} + +pub(crate) trait HasJavaClassIdent { + const JAVA_CLASS_IDENT: &str; } #[ext(JNIEnvExt)] diff --git a/crates/voicevox_core_java_api/src/synthesizer.rs b/crates/voicevox_core_java_api/src/synthesizer.rs index 98c4f02f0..9572c6ce6 100644 --- a/crates/voicevox_core_java_api/src/synthesizer.rs +++ b/crates/voicevox_core_java_api/src/synthesizer.rs @@ -107,8 +107,9 @@ unsafe extern "system" fn Java_jp_hiroshiba_voicevoxcore_Synthesizer_rsLoadVoice ) { throw_if_err(env, (), |env| { let model = env - .get_rust_field::<_, _, Arc>(&model, "handle")? + .get_rust_field::<_, _, Arc>(&model, "handle")? .clone(); + let model = model.read()?; let internal = env .get_rust_field::<_, _, Arc>>( &this, "handle", diff --git a/crates/voicevox_core_java_api/src/voice_model.rs b/crates/voicevox_core_java_api/src/voice_model.rs index 66bd3ca0a..38dcc2371 100644 --- a/crates/voicevox_core_java_api/src/voice_model.rs +++ b/crates/voicevox_core_java_api/src/voice_model.rs @@ -1,16 +1,20 @@ use std::{borrow::Cow, sync::Arc}; -use crate::common::{throw_if_err, JNIEnvExt as _}; +use crate::common::{throw_if_err, Closable, HasJavaClassIdent, JNIEnvExt as _}; use jni::{ objects::{JObject, JString}, sys::jobject, JNIEnv, }; +pub(crate) type VoiceModelFile = Closable; + +impl HasJavaClassIdent for voicevox_core::blocking::VoiceModelFile { + const JAVA_CLASS_IDENT: &str = "VoiceModelFile"; +} + #[no_mangle] -unsafe extern "system" fn Java_jp_hiroshiba_voicevoxcore_VoiceModelFile_00024Opened_rsOpen< - 'local, ->( +unsafe extern "system" fn Java_jp_hiroshiba_voicevoxcore_VoiceModelFile_rsOpen<'local>( env: JNIEnv<'local>, this: JObject<'local>, model_path: JString<'local>, @@ -20,24 +24,23 @@ unsafe extern "system" fn Java_jp_hiroshiba_voicevoxcore_VoiceModelFile_00024Ope let model_path = &*Cow::from(&model_path); let internal = voicevox_core::blocking::VoiceModelFile::open(model_path)?; - - env.set_rust_field(&this, "handle", Arc::new(internal))?; + let internal = Arc::new(Closable::new(internal)); + env.set_rust_field(&this, "handle", internal)?; Ok(()) }) } #[no_mangle] -unsafe extern "system" fn Java_jp_hiroshiba_voicevoxcore_VoiceModelFile_00024Opened_rsGetId< - 'local, ->( +unsafe extern "system" fn Java_jp_hiroshiba_voicevoxcore_VoiceModelFile_rsGetId<'local>( env: JNIEnv<'local>, this: JObject<'local>, ) -> jobject { throw_if_err(env, std::ptr::null_mut(), |env| { let internal = env - .get_rust_field::<_, _, Arc>(&this, "handle")? + .get_rust_field::<_, _, Arc>(&this, "handle")? .clone(); + let internal = internal.read()?; let id = env.new_uuid(internal.id().raw_voice_model_id())?; @@ -46,16 +49,15 @@ unsafe extern "system" fn Java_jp_hiroshiba_voicevoxcore_VoiceModelFile_00024Ope } #[no_mangle] -unsafe extern "system" fn Java_jp_hiroshiba_voicevoxcore_VoiceModelFile_00024Opened_rsGetMetasJson< - 'local, ->( +unsafe extern "system" fn Java_jp_hiroshiba_voicevoxcore_VoiceModelFile_rsGetMetasJson<'local>( env: JNIEnv<'local>, this: JObject<'local>, ) -> jobject { throw_if_err(env, std::ptr::null_mut(), |env| { let internal = env - .get_rust_field::<_, _, Arc>(&this, "handle")? + .get_rust_field::<_, _, Arc>(&this, "handle")? .clone(); + let internal = internal.read()?; let metas = internal.metas(); let metas_json = serde_json::to_string(&metas).expect("should not fail"); @@ -64,9 +66,19 @@ unsafe extern "system" fn Java_jp_hiroshiba_voicevoxcore_VoiceModelFile_00024Ope } #[no_mangle] -unsafe extern "system" fn Java_jp_hiroshiba_voicevoxcore_VoiceModelFile_00024Opened_rsDrop< - 'local, ->( +unsafe extern "system" fn Java_jp_hiroshiba_voicevoxcore_VoiceModelFile_rsClose<'local>( + env: JNIEnv<'local>, + this: JObject<'local>, +) { + throw_if_err(env, (), |env| { + env.take_rust_field::<_, _, Arc>(&this, "handle")? + .close(); + Ok(()) + }) +} + +#[no_mangle] +unsafe extern "system" fn Java_jp_hiroshiba_voicevoxcore_VoiceModelFile_rsDrop<'local>( env: JNIEnv<'local>, this: JObject<'local>, ) { diff --git a/crates/voicevox_core_python_api/Cargo.toml b/crates/voicevox_core_python_api/Cargo.toml index 94455e045..0c66d37ca 100644 --- a/crates/voicevox_core_python_api/Cargo.toml +++ b/crates/voicevox_core_python_api/Cargo.toml @@ -14,7 +14,6 @@ easy-ext.workspace = true futures-lite.workspace = true log.workspace = true once_cell.workspace = true -ouroboros.workspace = true pyo3 = { workspace = true, features = ["abi3-py38", "extension-module"] } pyo3-asyncio = { workspace = true, features = ["tokio-runtime"] } pyo3-log.workspace = true diff --git a/crates/voicevox_core_python_api/src/lib.rs b/crates/voicevox_core_python_api/src/lib.rs index 6c16cabff..db2f7471f 100644 --- a/crates/voicevox_core_python_api/src/lib.rs +++ b/crates/voicevox_core_python_api/src/lib.rs @@ -8,7 +8,6 @@ mod convert; use self::convert::{from_utf8_path, VoicevoxCoreResultExt as _}; use easy_ext::ext; use log::{debug, warn}; -use ouroboros::self_referencing; use pyo3::{ create_exception, exceptions::{PyException, PyKeyError, PyValueError}, @@ -16,6 +15,7 @@ use pyo3::{ types::{PyList, PyModule}, wrap_pyfunction, Py, PyObject, PyResult, PyTypeInfo, Python, }; +use voicevox_core::__internal::interop::raii::MaybeClosed; #[pymodule] #[pyo3(name = "_rust")] @@ -98,11 +98,6 @@ struct Closable { marker: PhantomData<(C, A)>, } -enum MaybeClosed { - Open(T), - Closed, -} - impl Closable { fn new(content: T) -> Self { Self { @@ -115,36 +110,15 @@ impl Closable { let lock = self .content .try_read_() - .map_err(|_| PyValueError::new_err(format!("The `{}` is closed", C::NAME)))?; - - return MappedLockTryBuilder::<'_, A::RwLock>, _, _, _> { - lock, - content_builder: |lock| match &**lock { - MaybeClosed::Open(content) => Ok(content), - MaybeClosed::Closed => Err(PyValueError::new_err(format!( - "The `{}` is closed", - C::NAME, - ))), - }, - } - .try_build(); - - // https://github.com/rust-lang/rust/issues/117108 - #[self_referencing] - struct MappedLock<'a, L: RwLock, T: 'static> { - lock: L::RwLockReadGuard<'a>, - - #[borrows(lock)] - content: &'this T, - } - - impl<'a, L: RwLock, T: 'static> Deref for MappedLock<'a, L, T> { - type Target = T; + .map_err(|_| PyValueError::new_err(format!("The `{}` is being closed", C::NAME)))?; - fn deref(&self) -> &Self::Target { - self.borrow_content() - } - } + voicevox_core::__internal::interop::raii::try_map_guard(lock, |lock| match &**lock { + MaybeClosed::Open(content) => Ok(content), + MaybeClosed::Closed => Err(PyValueError::new_err(format!( + "The `{}` is closed", + C::NAME, + ))), + }) } async fn close_(&self) -> Option { @@ -185,11 +159,12 @@ impl Drop for Closable { let content = mem::replace(&mut *self.content.blocking_write_(), MaybeClosed::Closed); if matches!(content, MaybeClosed::Open(_)) { warn!( - "デストラクタにより`{}`のクローズが行われました。通常は、可能な限り`{}`でクローズ\ - するようにして下さい", + "デストラクタにより`{}`のクローズが行います。通常は、可能な限り`{}`でクローズする\ + ようにして下さい", C::NAME, A::EXIT_METHOD, ); + drop(content); } } }