Conversation
|
@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.
ℹ️ 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
| SharedMutexNameStrategy.UriEscape or SharedMutexNameStrategy.Default => Uri.EscapeDataString(Path.GetFullPath(fileName).ToLowerInvariant()), | ||
| SharedMutexNameStrategy.Sha1Hash => Sha1(fileName), |
There was a problem hiding this comment.
The Sha1Hash branch no longer normalizes the file path the way the legacy code did. The previous implementation hashed Path.GetFullPath(settings.Filename).ToLower() so the mutex name was identical regardless of relative versus absolute paths or path casing. Here the raw fileName is hashed, so two processes that point at the same file with different path representations will compute different mutex names and both acquire their own mutexes, effectively removing mutual exclusion and defeating the purpose of the compatibility switch.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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
| private static string CreateUsingUriEncoding(string fileName) | ||
| { | ||
| var normalized = Path.GetFullPath(fileName).ToLowerInvariant(); | ||
| return Uri.EscapeDataString(normalized); |
There was a problem hiding this comment.
Switching the default mutex name generation to CreateUsingUriEncoding encodes the entire absolute path rather than hashing it. On Windows a named mutex cannot exceed 260 characters, but an escaped full path grows by two extra characters for every directory separator and other reserved characters. Valid database files whose paths are already near MAX_PATH now cause new Mutex(false, …) to throw ArgumentException, whereas the previous SHA1 hash was always 40 characters. This makes some databases unusable when their path length is only moderately long.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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
| private static string CreateUsingUriEncodingWithFallback(string fileName) | ||
| { | ||
| var normalized = Normalize(fileName); | ||
| var uri = Uri.EscapeDataString(normalized); | ||
|
|
||
| if (IsWindows() && | ||
| uri.Length + CONSERVATIVE_EXTERNAL_PREFIX_LENGTH > WINDOWS_MUTEX_NAME_MAX) | ||
| { | ||
| // Short, stable fallback well under the limit. | ||
| return "sha1-" + ComputeSha1Hex(normalized); | ||
| } | ||
|
|
||
| return uri; |
There was a problem hiding this comment.
The default mutex name builder only switches to a SHA‑1 hash when the URI-encoded path exceeds 250 characters and the runtime is Windows. However SharedMutexFactory.Create passes the name to new Mutex(false, fullName) on every platform, and the POSIX implementation also rejects names longer than 251 bytes. With a moderately long database path (≈90+ characters after encoding the / separators) the Linux/OSX runtime will throw ArgumentException: name too long, making SharedEngine unusable for that file. The previous SHA‑1 approach always produced a short, safe name. Consider applying the length check on all platforms or hashing when the encoded value exceeds the POSIX limit as well.
Useful? React with 👍 / 👎.
2 participants
Replaces #2557