Make the history file mutext name unique across sessions

[Dongbo Wang (daxian-dbw)]

GetHistorySaveFileMutexName() uses GetHashCode() to construct the mutex name.
GetHashCode() in .NET Core returns a value that is unique in the current process only, and it results in different mutex name being used for different pwsh sessions for the history file.

The FNV-1a hashing algorithm is used (32-bit) to provide a consistent hash code across different pwsh sessions.

[Jason Shirk (lzybkr)]

Rather than hashing, we might as well just use the filename directly. There is a limit (260) in the mutex name, so if the name is too long, I'd drop the leading part of the filename, it's likely not unique. For example, c:\Users\jason\AppData\ seems perfectly safe to drop and have reasonable uniqueness.

[Dongbo Wang (daxian-dbw)]

Backslash \ is a reserved character for mutex name, so using file name means we need extra work to normalize the path separator.
What is the disadvantage of using hashing? It's a simple hashing algorithm. There is possibility of collision, but should be rare.

[Jason Shirk (lzybkr)]

CRC32 is actually pretty bad, I've seen collisions in practice.

But I'm mostly objecting to needing to copy the code and hoping for fewer lines of code.

There is also a benefit to using the filename - it should be more obvious what the mutex is protecting if you were using low level tools like the module NtObjectManager:

#137 PS> Get-NtMutant PSReadLineHistoryFile_1095088493 | fl *

Owner                 : PID: 0 - TID: 0
CurrentCount          : 1
OwnedByCaller         : False
AbandonedState        : False
GrantedAccess         : QueryState, Delete, ReadControl, WriteDac, WriteOwner, Synchronize
FullPath              : \Sessions\1\BaseNamedObjects\PSReadLineHistoryFile_1095088493
GrantedAccessMask     : 2031617
SecurityDescriptor    : O:BAG:DUD:(A;;0x1f0001;;;SY)(A;;0x1f0001;;;BA)(A;;0x1f0001;;;S-1-5-5-0-16814681)
Sddl                  : O:BAG:DUD:(A;;0x1f0001;;;SY)(A;;0x1f0001;;;BA)(A;;0x1f0001;;;S-1-5-5-0-16814681)
Handle                : 0x108C
NtTypeName            : Mutant
NtType                : Name = Mutant - Index = 17
Name                  : PSReadLineHistoryFile_1095088493
CanSynchronize        : True
CreationTime          : 12/31/1600 4:00:00 PM
AttributesFlags       : None
HandleReferenceCount  : 2
PointerReferenceCount : 65491
Inherit               : False
ProtectFromClose      : False
Address               : 0
IsContainer           : False

[Dongbo Wang (daxian-dbw)]

OK, got your concern. I will update the PR to not rely on the CRC32 hashing function.

[Jason Shirk (lzybkr)]

If you did want to stick with hashing, I'd use something like fnv-1a instead, it's very little code.

[Dongbo Wang (daxian-dbw)]

Yes, personally I still want to use a hash algorithm for the unique name purpose. I will use fnv-1a instead.

[Keith Hill (rkeithhill)]

I suppose using a generated GUID would be too long?

[Dongbo Wang (daxian-dbw)]

The GUID needs to be generated based on the history file path, so that when the path is changed in a pwsh session, a different mutex name is used to guard that file.
Generating a GUID out of a file name is basically hashing again.

[Dongbo Wang (daxian-dbw)]

Jason Shirk (@lzybkr) The PR has been updated to use the FNV-1a hashing algorithm. PR description is also updated. Thanks for the suggestion!

[Steve Lee (SteveL-MSFT)]

LGTM

[Jason Shirk (lzybkr)]

I was going to suggest avoiding the conversion to byte array (can we use a span somehow?) but then realized we probably should be normalizing the name anyway, e.g. ensuring consistent case on Windows.

[Dongbo Wang (daxian-dbw)]

we probably should be normalizing the name anyway

Agreed. Update Set-PSReadLineOption to sort of resolve (not really) PSPath. Also call ToLower before sending the file path to hash method.

avoiding the conversion to byte array (can we use a span somehow?)

This is a fun exercise!
We cannot use span because we need to compile it against .NET 4.6.1, but we don't really need to use a span here.
The update ComputeHash(string input) behaves exactly the same as the ComputeHash(string input) implementation in the following code

internal static uint ComputeHash(byte[] buffer)
{
    uint hash = FNV32_OFFSETBASIS;
    for (int i = 0; i < buffer.Length; i++)
    {
        hash = (hash ^ buffer[i]) * FNV32_PRIME;
    }
    return hash;
}

internal static uint ComputeHash(string input)
{
    return ComputeHash(Encoding.Unicode.GetBytes(input));
}