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

RustのパニックをC++の例外などに変換して送出するAPIを作る #561

Open
3 tasks done
qryxip opened this issue Aug 2, 2023 · 3 comments
Open
3 tasks done

Comments

@qryxip
Copy link
Member

qryxip commented Aug 2, 2023

内容

C APIに次の関数を追加します。

void voicevox_set_rust_panic_translator(void (*translator)(const *VoicevoxRustPanicInfo));

この関数はRustのパニックの情報であるVoicevoxRustPanicInfoから、C++の例外などに変換するコールバックを登録します。そしてVOICEVOX COREの実装から万が一Rustのパニックが出た場合にそのパニック自体ではなく、変換されたC++の例外(など)が発されるようになります。

Pros 良くなる点

  • RustのパニックをC++等の言語で受け取れるようになる。

Cons 悪くなる点

実現方法

C APIのextern "C"extern "C-unwind"にした(#541)後、

  1. panic=unwindにする。

  2. voicevox_set_rust_panic_translatorを実装。

  3. Rustのパニックが発生しうる場所をstd::panic::catch_unwindで包み、"catch"したパニックを上記のvoid (*translator)(const *VoicevoxRustPanicInfo)に入れる。

    多分これを加工して使うことになる。

    pub(crate) fn into_result_code_with_error(result: CApiResult<()>) -> VoicevoxResultCode {

    translatorがunwindしなかった場合、std::process::abort

VOICEVOXのバージョン

N/A

OSの種類/ディストリ/バージョン

  • Windows
  • macOS
  • Linux

その他

@Hiroshiba
Copy link
Member

issue作成ありがとうございます!!

なるほどです、確かに例外を置き換えられるとより強固なライブラリになる気がしました。
ただどういう実装にすれば良いのかが全く検討つかないというのがちょっと正直なところです。

このあたりって何か前例あったりするのでしょうか?
というのも、例えばC++以外の言語に例外を投げたい場合にどうなっていると嬉しいのかとか、先見の明を持っているかもしれないので参考になりそうだなと思った次第です。

@qryxip
Copy link
Member Author

qryxip commented Aug 4, 2023

DとかGoとかだとあるかもしれません。まだそのあたりまでは調べていません。

それ以前に、現在のRustだと #556 と両立できないことに気付いてしまいました。RustのパニックとC++の例外、両方の発生を同時に許容することはできません。

The behavior of catch_unwind when a foreign exception encounters it is currently left undefined.

This constraint is partially met: the behavior of foreign exceptions with respect to catch_unwind is currently undefined, and left for future work.

このissueの機能と #556 、どちらが有用かというと #556 かと思います。まあパニックは回復不可能なエラーという位置付けなので、現状のプロセス強制終了の挙動でもよさそうな気がします。

で、このissueをどうするかですが、上記の"future work"がするっと成し遂げられて半年後とか3ヶ月後には利用可能になっている、ということも十分考えられるんですよね... なので存続はさせたいと思っています。

@Hiroshiba
Copy link
Member

Hiroshiba commented Aug 5, 2023

なるほどです!

両立できない

ちょっとちゃんとわかってないんですが、コード上両立はできるけれども、例外の発生のことを考えると危ないっていう感じですよね。
個人的にはまあその状態でも両方とも実装しちゃって、その中で例外が発生しちゃうとどうなるか分かりませんよ(=やめてください)という案内にするというのも全然ありかなとか思いました!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants