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

windowsでエンジンの多重起動を可能にする(openjtalkによるエラーを出なくする) #1347

Open
Hiroshiba opened this issue May 28, 2024 · 16 comments
Labels
機能向上 状態:設計 設計をおこなっている状態 要議論 実行する前に議論が必要そうなもの

Comments

@Hiroshiba
Copy link
Member

内容

windows環境で、エンジンを2つ起動するとopenjtalk周りでエラーが出ます。
pyopenjtalk.set_user_dict`したものはopenjtalk側でずっとファイルハンドラが必要で、同じファイルに書き込めないためです。

pyopenjtalk.set_user_dict(str(compiled_dict_path.resolve(strict=True)))

この辺りを、ユーザーにバグだと思われることなく解決したいです。

Pros 良くなる点

エディタ側でエンジンがなぜか消えずに残っていることが結構あり、エディターを起動した時にエンジンがエラーになるのを防げる。
あと開発段階でエンジンを複数起動できるようになる。

Cons 悪くなる点

完璧な実装方法が思いついてない

実現方法

コンパイルしたユーザー辞書をset_user_dictしてるので、そのコンパイル済みの辞書のパスをランダムにすれば一応問題は解決されるはず。
ただそうすると辞書を更新するたびに、あるいはエンジンが起動するたびに新しいファイルが生まれて残り続けてしまいます。

迂回作としては、ちょっと雑なアイデアだけど、compiled-0.diccompiled-4.dicまで空いてるパスを探すとか・・・?

その他

#1332 (comment) で整理したメモをこちらにも転機します:

  • 目指す UX
    • ユーザーが意図していない挙動になっているのに、エラーや通知がないのは避けたい
    • エンジンの多重起動でエラーになるのも避けたい
    • ↑のUXが衝突する場合どっちが優先の高いかはエラー次第
  • 挙動の整理
    • update_dict、つまり保存した辞書などをopenjtalkに読み込ませる部分で、多重起動している場合にエラーになる
      • pyopenjtalk.set_user_dictしたものはopenjtalk側でずっとファイルハンドラが必要で、同じファイルに書き込めないから
    • 以前握りつぶすコードがあったが、今はなくなってる
  • 多重起動時の読み込みエラーを握りつぶすべきか
    • もし握りつぶした場合の意図しない挙動は「エンジンが2つ起動している時、辞書登録できたのに辞書が反映されない」になる
      • 後から起動したエンジンに対してVOICEVOX用の辞書には登録できるけど、openjtalk用の辞書には反映されない&エラーにもならないので、登録できたのに反映されない形になる
    • でも辞書登録しない人には関係ない
    • 判断つかない。。。迂回方法を探りたい。
@Hiroshiba Hiroshiba added 機能向上 要議論 実行する前に議論が必要そうなもの labels May 28, 2024
@Hiroshiba
Copy link
Member Author

Hiroshiba commented May 28, 2024

@takana-v @sabonerune ちょっとメンションすみません!!
コンパイル済みのユーザー辞書のパスを5つほど用意して順に空いてるの使う感じにすれば、エンジンが多重起動しない問題をそこそこ綺麗に解決するのではと思ったのですが、なんか認識間違ってる気がちょっとしてます 😇
変なとこあったらご指摘いただけると。。。

@sevenc-nanashi
Copy link
Member

sevenc-nanashi commented May 28, 2024

ただそうすると辞書を更新するたびに、あるいはエンジンが起動するたびに新しいファイルが生まれて残り続けてしまいます。

FastAPIにteardown的なフックってないんですかね?それで消すとかどうでしょう。(AIVoiceVoxだとctrl-cのフックで後処理をしていてちゃんと動いているので、多分FastAPIのteardownも呼ばれるはず)

@sabonerune
Copy link
Contributor

FastAPIにteardown的なフックってないんですかね?

https://fastapi.tiangolo.com/advanced/events/#lifespan

@asynccontextmanager
async def lifespan(app: FastAPI) -> AsyncIterator[None]:
user_dict.update_dict()
yield

lifespanで終了時のクリーンアップができます(yieldの後の行に処理を書く)

しかしエディタはnode-tree-killを使用しているため実行されません。


起動時にコンパイル済みの辞書を書き込むディレクトリを空にするという手もあると思います。
辞書の切り替え時にタイミング悪く削除されるリスクもありますがまあ、無視できるでしょう。

#620 (comment)
ユーザー辞書がコンパイルできなくなったとき起動できなくなる?

POSIXでもuser_dict.jsonの操作がアトミックではなかったり他のエンジンが辞書操作をするとpyopenjtalkが適用中の辞書とAPIが返す辞書の内容が異なるようになったりするので多重起動が可能と言える状態ではない気が

@Hiroshiba
Copy link
Member Author

@sevenc-nanashi
もちろんteardown的なとこで消せばよいのですが、なんかこう、electronがむずすぎて「終了時にシグナルを正確に呼ぶ」ってのが意外と大変そうなんですよね・・・ 😇

それプラスtmpディレクトリに保存する形であればまあ残っちゃっても良いかって感じなんですが、次は環境によってはtmpディレクトリへのファイルmvができずatomic操作ができないという問題が・・・・・・・・

あれ、もしかしてatomic操作いらなくなった????
プロセス開始時にランダムなtmpファイルを決めて書き込む形ならatomic操作不要かも・・・・・?
update_dict自体をそのプロセス内で排他制御しないといけないですが。


@sabonerune

起動時にコンパイル済みの辞書を書き込むディレクトリを空にするという手もあると思います。

自分もこれ考えたんですが、わざと2つ起動する人とかもいる気がするんですよね〜・・・。
Linuxとかだとファイル開き中でもファイル削除できちゃうわけで、そうすると起動済みだったエンジンがなぜかユーザー辞書使えない形になるとかあるんじゃないかなぁと。
なので避けられるなら避けたい系のイメージでした。

@Hiroshiba
Copy link
Member Author

Hiroshiba commented May 28, 2024

今のとこまとめると、こう?

今のmainの実装
→windowsが多重起動できなくて不便。まあまあ避けたい。

OSErrorを握りつぶす場合
→Windowsで2つ目に起動したエンジンは、ユーザー辞書登録ができるけど反映されない形になる。わりと避けたい。

コンパイル済み辞書の名前をN個用意し、1つずつ使えるか試していく
→N個より増えるとエラーになる。まあまあ避けたい。

コンパイル済み辞書の名前をランダムにし、teardownで消す
→エディタからteardownを正確に呼ぶ自信がない。まあでもアリかも。

コンパイル済み辞書の名前をランダムにし、エンジン起動時に全ファイル消す
→Linuxでエンジンを2つ起動すると、1つ目の方でユーザー辞書が使えなくなるかも。まあまあ避けたい。

コンパイル済み辞書の名前をランダムにし、tmpディレクトリに保存して、teardownで消す
→エディタからteardownを呼ぶ自信がないけど、まあtmpディレクトリだから許されそう。一番アリかも。

@sabonerune
Copy link
Contributor

@Hiroshiba

Linuxとかだとファイル開き中でもファイル削除できちゃうわけで、そうすると起動済みだったエンジンがなぜかユーザー辞書使えない形になるとかあるんじゃないかなぁと。

Linuxの場合開いているファイルを削除しても問題ないのではと思います。
Linuxの場合開かれているファイルを削除してもファイルパスから参照できなくなるだけで引き続き操作はできるという感じだったような気がします。

@Hiroshiba
Copy link
Member Author

@sabonerune まあ大丈夫かもなのですが、あんまりOSやライブラリやファイルシステムの(隠れ)仕様に頼った実装にしない方が良いかなぁと。
どうしても避けられないなら通らないとダメだけど、避けられる方法があるならわざわざ通る必要はないかなって感じです。


とはいえエディタ側にsignal実装もちょっと大変そうではあるんですよねぇ。。
うーーーーーん。候補ファイル全部消すのはちょっと怖い気もするし、かといって連番にして0から使ってくのは同じファイルにあたって変な挙動しかねなさそう。

うーーーーーーーーーん。ラウンドロビンもどきとかぁ・・・?
N個ファイル名候補用意して、更新時間でソートして、古いやつから使えるか試していくみたいな・・・。

@sevenc-nanashi
Copy link
Member

sevenc-nanashi commented May 29, 2024

しかしエディタはnode-tree-killを使用しているため実行されません。

これ本当です?AIVoiceVoxのteardown(ctrl-cハンドル)はちゃんと動いてるので動くと思いますが

もちろんteardown的なとこで消せばよいのですが、なんかこう、electronがむずすぎて「終了時にシグナルを正確に呼ぶ」ってのが意外と大変そうなんですよね・・・ 😇

tree-killってシグナル指定できませんでしたっけ?一回SIGINT送って10秒後にSIGKILLみたいなことできそうな気が。

@Hiroshiba
Copy link
Member Author

Hiroshiba commented May 29, 2024

@sevenc-nanashi とりあえずコード追っかけてみました。

エディタ側ではtreeKillを呼んでいて、
https://github.com/VOICEVOX/voicevox/blob/a07c776956987f8a44538089278449cd0469f022/src/backend/electron/manager/engineManager.ts#L504
treeKill側はwindowsは即強制終了のコマンド(たぶん)、linuxとかはSIGTERMが呼ばれてそうでした。
https://github.com/pkrumins/node-tree-kill/blob/cb478381547107f5c53362668533f634beff7e6e/index.js#L29
https://github.com/pkrumins/node-tree-kill/blob/cb478381547107f5c53362668533f634beff7e6e/index.js#L82

なのでwindowsなら即強制終了・・・という理解で合ってるはずなんですが、まだ未知ななにかがあるかもですねぇ。
今もなおエンジン側のteardownで拾えるシグナルが飛ばされてるのであれば、teardown実装するのはかなりアリ。

@sevenc-nanashi
Copy link
Member

なるほどー。

VOICEVOX/voicevox#1540

となるとこれを再Openしてもいいかも?

@tarepan tarepan added the 状態:設計 設計をおこなっている状態 label Jun 1, 2024
@Hiroshiba
Copy link
Member Author

@sevenc-nanashi VOICEVOX/voicevox#1540 は・・・うーん、1つのWebAPIとしては強力すぎると思うんですよね・・・。
なにか前例があれば参考にしたいです。特に無さそうであればコメントをオフトピックにしても良いかも?

@Hiroshiba
Copy link
Member Author

とりあえず候補が決まってない状況なのが一番もったいない気がしました!
個人的にはコンパイル済み辞書の名前をランダムにし、tmpディレクトリに保存して、teardownで消すが最良に思いました。

まず現状、エディタはどうやらSIGTERMをエンジンに送っているっぽいことがわかっているためです。
エディタで使っているtreekillはmacos/linuxならSIGTERMを送るし、windowsでも送られているっぽいためです。(後者はなぜかはわかりませんが・・・)

あとこの方法であれば、最悪問題がほぼ無いと思います。
エラーが起きることが原理上ない、というのが最大のメリットかなと。
問題があるとすれば、teardownが呼ばれなくてtmpディレクトリが肥大化していくことくらいですが、そもそもコンパイルされたユーザー辞書は相当小さいので問題にならないよなーと。

設計としてはこうでしょうか?

  1. pyopenjtalk.set_user_dictにセットするcompiled_dict_pathのパスをtmpディレクトリ内のランダムなパスにする
  2. Path.replaceを呼ぶために、pyopenjtalk.create_user_dictで作るtmp_compiled_pathも同じディレクトリ内にする
  3. lifespan内で辞書の終了処理をする
    • pyopenjtalk.unset_user_dictを実行してcompiled_dict_pathを削除するfinalize_dict関数を作って叩く感じ

@sabonerune
Copy link
Contributor

#1248 (comment)
Uvicornの仕様によりSIGINT以外を受け取った場合はgraceful shutdownになりません。

POSIXの方はTreeKillのシグナルをSIGINTにすれば(恐らく?)解決できますが、WIndowsではそれは不可能です。
(SIGINT送るときはTreeKillを使わない方がいい気がするが)

@Hiroshiba
Copy link
Member Author

@sabonerune
uvicornのコード読んでみたのですが、もしかしたらSIGINT以外でもgracefulシャットダウンしてるかも・・・?

ここでSIGINTもTERMもBREAKも同じ用にhandle_exit関数を呼ぶようにされていて、
https://github.com/encode/uvicorn/blob/c23cd24e6676ee0638d014d7000af1e6e0996bd6/uvicorn/server.py#L318

handle_exit関数は同じ処理をしているのでそう思いました。(SIGINTだけ2回目は強制終了?)
https://github.com/encode/uvicorn/blob/c23cd24e6676ee0638d014d7000af1e6e0996bd6/uvicorn/server.py#L330-L335

実際linux環境でSIGTERM(たぶんあたい何もなくkillすればよいだけ)してuvicornがgraceful shutdownしてそうか確かめると良さそう?

@sabonerune
Copy link
Contributor

@Hiroshiba
WindowsでもSIGBREAK(Ctrl + Break)で同じことができるので確認したところgraceful shutdownするみたいです。失礼しました。

ただ、taskkill /pid [pid] /T /Fで強制終了した場合は呼ばれないみたいです。

@Hiroshiba
Copy link
Member Author

@sabonerune

ただ、taskkill /pid [pid] /T /Fで強制終了した場合は呼ばれないみたいです。

以前の話だとなぜか呼ばれたという感じだった記憶がありますが、まあ、呼ばれない前提で考えるのが良さそう。
このあたりはWindowsのOSのバージョンによっても変わりそう。

そうなると #1347 (comment) の方法ではtmpに辞書が残り続けてしまう前提で良さそう。流石にそれは微妙そうに感じます。

とりあえずこのissueの解決策としては、ワークアラウンドですが↓の方法が良さそうかなと思いました。

コンパイル済み辞書の名前をN個用意し、1つずつ使えるか試していく

候補のファイルを5つくらい持っておいて、順に開いていって開けなかったら次のを試す、という感じをイメージしています。
これだとWindowsで6個目のエンジンを起動するとエラーになる問題がありますが、まあワークアラウンドということで・・・。

別件で、正常終了させる方法はほんとに難しいですね。。。
ちょっと終了方法のissueにコメントしてみます。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
機能向上 状態:設計 設計をおこなっている状態 要議論 実行する前に議論が必要そうなもの
Projects
None yet
Development

No branches or pull requests

4 participants