fix(settings): 設定の保存の排他を対話セッションをまたぐ名前にする - #80
Conversation
設定の保存のクロスプロセス排他は、接頭辞の無い名前を使っていた。コメントは 「settings は %AppData% 配下なのでユーザ毎にしか共有されない」を理由に挙げて いたが、この前提が誤っている。 %AppData% はユーザ毎に分かれるが、同じユーザの 2 つの対話セッション (ユーザーの切り替えやリモートデスクトップ) からは同じ場所を指す。一方で接頭辞 の無い名前はセッションごとの名前空間に作られるため、そこで並走した保存を直列化 できない。両セッションが同じ settings.json を read-modify-write すると、片方の 更新が黙って潰される。 名前を Global\ にした。守る相手ごとに分けるため、設定ファイルのパスから引いた 鍵を足す。1 つにまとめると、別のユーザどうしや、パスを指定して動かしている相手 (テストなど) まで待ち合わせる。 Global\ を作れない環境向けのフォールバックは #48 が既に持っていたので、鍵の 作り方と一緒に GlobalMutex へ寄せ、UpdateStage もそちらを使うようにした。 作れなかったことは覚えない。UnauthorizedAccessException は「この環境では作れ ない」だけでなく「その名前のものが既にあり、開く権利が無い」でも飛ぶ。名前ごと の事情を環境の事情として覚えると、以後どの名前もセッション内だけの名前へ落ちて、 黙って守れなくなる。 Refs #52
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf73f28b69
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ScopeKeyOf が見ているのはファイルではなく文字列なので、C:\dir\settings.json と C:\dir\.\settings.json のように OS が同じ場所へ解決する綴りでも、そのままでは 別の鍵になっていた。別の鍵は別のロックであり、同じ資源を守っているつもりで 守れていない。設定の保存では、これが read-modify-write の取りこぼしに戻る。 Path.GetFullPath で絶対パスに直してから縮める。区切りの違いと . / .. も ここでそろう。既に絶対パスで正規化済みのもの (AppContext.BaseDirectory から 引くインストール先) には効かないので、置き換え待ちの置き場所の名前は動かない。 Refs #52
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aeb212d52c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
名前を変えた結果、この版は旧版の Mutex を見なくなった。同じ対話セッションで 別の場所の旧版と同じ settings.json を保存すると、名前を変える前には効いていた 直列化が効かず、read-modify-write の取りこぼしに戻る。旧名も「旧名 → 新名」の 順で取り、旧版が残っている間の待ち合わせを保つ。新名を取るのはこの版だけで、 この版は必ず旧名から取るので、順の食い違いは起きない。 もう 1 点、断られたときに別の名前へ落ちるのが早すぎた。 UnauthorizedAccessException は「作れない環境」だけでなく「その名前のものが既に あり、全部の権利では開けない」でも飛ぶ。後者でセッション内だけの名前へ落ちると、 先客は Global\ の物を、こちらは別の物を、それぞれ同時に持ててしまう。ロックを 持っているつもりで誰とも待ち合わせていない。Mutex.OpenExisting は待ち合わせに 要る権利だけを求めるので、まずこれで開き直す。 開くこともできなかったときだけ、これまで通りセッション内だけの名前へ落ちる。 そこは守り切れていない。投げると保存そのものができなくなり、並走していなくても 書けない側の損が確実に出る。落ちた側は、並走したときだけ失う。 Refs #52
2 participants
issue #52 の残りへの対応。3 点のうち、データを失う 1 点だけを直し、残り 2 点は理由を書いて受け入れる。
直すもの: 設定の保存の名前空間
SettingsStoreのコメントはこう書いていた。>
Global\は付けずユーザセッション内のみ排他にする (settings は%AppData%配下なのでユーザ毎にしか共有されない)この前提が誤っている。
%AppData%はユーザ毎に分かれるが、同じユーザの 2 つの対話セッション（ユーザーの切り替えやリモートデスクトップ）からは同じ場所を指す。一方で接頭辞の無い名前はセッションごとの名前空間に作られるため、そこで並走した保存を直列化できない。両セッションが同じsettings.jsonを read-modify-write すると、片方の更新が黙って潰される。#52 で唯一データを失う経路がここである。
名前を
Global\にした。守る相手ごとに分けるため、設定ファイルのパスから引いた鍵を足す。1 つにまとめると、別のユーザどうしや、パスを指定して動かしている相手（テストなど）まで待ち合わせる。Global\を作れない環境向けのフォールバックは #48 が既に持っていたので、鍵の作り方と一緒にGlobalMutexへ寄せ、UpdateStageもそちらを使うようにした。作れなかったことは覚えない。
UnauthorizedAccessExceptionは「この環境では作れない」だけでなく「その名前のものが既にあり、開く権利が無い」でも飛ぶ。名前ごとの事情を環境の事情として覚えると、以後どの名前もセッション内だけの名前へ落ちて、黙って守れなくなる。例外の往復より、取り違えないことを採った。鍵を作る前に綴りをそろえる (aeb212d)
鍵は文字列から引くので、
C:\dir\settings.jsonとC:\dir\.\settings.jsonのように OS が同じ場所へ解決する綴りでも、そのままでは別の鍵になる。別の鍵は別のロックであり、同じ資源を守っているつもりで守れていない。 設定の保存では、これが元の read-modify-write の取りこぼしに戻る。Path.GetFullPathで絶対パスに直してから縮める。区切りの違いと./..もここでそろう。正規化はScopeKeyOfの中に置いたので、同じ鍵を使うUpdateStage側 (App はAppContext.BaseDirectoryから引いた絶対パス、ヘルパはそれを--targetで受け取る) の綴りのずれも一緒に閉じている。既に正規化済みの絶対パスに対しては
Path.GetFullPathは同じ文字列を返すため、既存のインストールの置き場所 (%AppData%\VRCToolsDataSync\update\<鍵>) は動かない。受け入れるもの 1: 多重起動の抑止は現状のまま
Global\にすると、2 つ目のセッションではアプリを起動できなくなる。動く機能を取り上げて、より軽い問題を直すことになる。 issue 本文もこれを「利用者から見ると壊れている」と書いている。加えて #78 で、「ウィンドウの無いプロセスに抑止を掴まれて抜けられない」状態そのものが消えた。急ぐ理由も無い。
その帰結として、issue の 2 点目（置き換えの最中に別セッションの App が旧
app\を掴み、ヘルパのリネームが失敗する）は残る。ただしUpdateInstallerが巻き戻し、取得は残るので次の機会に適用し直される。壊れる方向の失敗ではない。受け入れるもの 2:
Discard()がincoming.zipも消す局所的には直せない。
Discard()は適用のロックの下で走る。その中で取得のロックを取ると「適用 → 取得」の順になり、取得の側が「取得 → 適用」で取る（昇格のため）のと逆になってデッドロックする。直すには「どのファイルをどちらのロックが持つか」の組み替えが要り、#48 の中断からの復旧の判定（横に残った記録を ZIP と突き合わせて仕上げる経路）に触れる。失うのは取得のやり直し 1 回で、
.oldや設定を失う類ではない。検証
.を挟んだ綴り、..を含む綴り、区切りを/にした綴りGlobalMutex.Createで作ったものが別スレッドから同じものとして見えることRefs #52, #48
Generated by Claude Code