Open chrisant996 opened 1 day ago
chrisant996
commented
1 day ago P.S. I'm working on adding support in Clink for easily switching back and forth between custom prompts, and I plan to include an oh-my-posh.clinkprompt module out of the box. I'm working on that module right now, which is the context under which I ran into the omp.cache problem.
lewis-yeung
commented
1 day ago @chrisant996 Since v22.3.0, cache files are split into session level (named as omp.cache.\template_cache_<SESSION_ID> are left in the omp.cache file, they should have been no longer picked up by newer versions of OMP.
@JanDeDobbeleer As there is a chance that a session cache file named after PID (omp.cache.\
chrisant996
commented
1 day ago @lewis-yeung I recall originally it tried to use guids on Windows, but the guids were just random numbers, which are really pseudorandom numbers. And pseudorandom numbers are not really guids at all. They were getting reused and colliding often. I believe I'm the person who reported that problem originally, which led to using process ids instead. But process ids are not guid-like.
If you think it's robust and easy to implement guids that uniquely identify a process on Windows, I'd like to hear more about how. I'm not aware of any way to do that and satisfy both of the core requirements:
Maybe omp isn't finding the entries in omp.cache, maybe it's finding them in old omp.cache.SESSION_ID files. I don't know for sure which of those is happening, but the latest version of omp downloaded today is definitely still experiencing this problem.
Session ids don't have guid-like semantics, and can't be used as long term identifiers. When I had suggested using process id instead of unseeded pseudorandom number (which was always the same "random" number), I didn't realize that the id would be used as a long term identifier. I'm not aware of any way to have a reliable long term unique identifier of a process in Windows, so if you think you know a solution, I'd love to hear more.
JanDeDobbeleer
commented
1 day ago @chrisant996 I do think there's an issue in the cleanup of the cache in that case. I will fix that in #5414 where the caching of the environment variables is also removed. It also shouldn't load all that data, it's waste.
lewis-yeung
commented
1 day ago @chrisant996
They were getting reused and colliding often.
I know they're pseudorandom numbers, but still almost impossible to be duplicated in daily use. Maybe I'm dumb, but I don't quite understand why you mentioned that they collide OFTEN, even with 128-bit length?
The same guid must be generated repeatedly for the same process session (of the parent shell, not of omp).
The session ID will have been written into the environment POSH_PID after the initialization script is sourced in a CMD session, and later read to match the corresponding cache file whenever the OMP executable is called. So, there is no need to generate a GUID repeatedly for the same process.
chrisant996
commented
1 day ago They were getting reused and colliding often.
I know they're pseudorandom numbers, but still almost impossible to be duplicated in daily use. I don't quite understand why you mentioned that they collide OFTEN, even with 128-bit length?
@lewis-yeung But when there is zero entropy, the number of bits is irrelevant. 😉
Refer to https://github.com/JanDeDobbeleer/oh-my-posh/issues/5182#issuecomment-2197452518.
Pseudorandom number generators produce the exact same sequence of numbers every single time, for a given seed. The default seed is a hard-coded constant. Omp did not apply a different seed. So, literally every single time it generated a guid, it generated the exact same guid.
But suppose omp wanted to apply a seed. Even if there's high entropy in the initial seed, the initial seed is only 32 bits. You can't get a full 128 bits of entropy from only 32 bits.
But pseudorandom numbers are only as strong as the initial seed. May as well skip calling rand() and simply use the initial seed itself as the GUID. So, how to pick a seed that has 128 bits worth of entropy in practice?
The entropy is what matters: you can produce as many bits as you want -- 256, 512, 4096, ... -- but if the math isn't sound, it will just be a very bloated representation of a much smaller range of actually reachable values. E.g. if you count from 0 to 1000 incrementing by 100, only 11 values are reachable even though the upper board of 1000 makes it sound like there are 1001 reachable values. The same kind of problem happens when trying to generate 128 bits worth of entropy from only 32 bits of input, or when using poor quality seeds.
Reading up on the pitfalls of pseudorandom number generators is interesting. For example, using them in card games is problematic because a large proportion of possible deck shuffles are mathematically unreachable (now, imagine if pseudorandom numbers were used in online gambling...problematic indeed!). Random number generation is a more nuanced and difficult topic than it might appear on the surface.
The same guid must be generated repeatedly for the same process session (of the parent shell, not of omp).
The session ID will be written into the environment
POSH_PIDonce the initialization script has been sourced in a CMD session, and later read to match the corresponding cache file whenever the OMP executable is called. So, there is no need to generate a GUID repeatedly for the same process.
That gets tricky because environment variables are inherited. So when a first cmd.exe process spawns a second cmd.exe process, the second one inherits the POSH_PID from the first one. Checking whether POSH_PID already exists would not be sufficient.
But, maybe the requirements can be relaxed: Maybe it's ok to create a new GUID whenever the omp Lua script gets reloaded in the session? That won't be often, but it can definitely happen multiple times in a single cmd.exe process session. If omp can tolerate the POSH_PID periodically changing in a given session, then that would greatly simplify the problem.
But it needs to be some a real GUID from a library with strong guarantees, not a homegrown identifier. For example UuidCreateSequential or UuidCreate or CoCreateGuid would be suitable.
chrisant996
commented
1 day ago In fact, I'm going to add Lua APIs in Clink to easily access UuidCreateSequential and UuidCreate, to efficiently and reliably produce proper GUIDs from inside Clink.
lewis-yeung
commented
1 day ago @chrisant996 Ah, I see, but the GUIDs I was talking about are real GUIDs/UUIDs (e.g., generated by APIs from a Go package, which would be reliable enough), not the implementation using math.random before #5183. Sorry for any confusion and thanks for providing details under the hood. I did learn something. :)
So when a first cmd.exe process spawns a second cmd.exe process, the second one inherits the POSH_PID from the first one. Checking whether POSH_PID already exists would not be sufficient.
That doesn't matter. OMP sets a unique POSH_PID (via the initialization script loaded) for each new CMD session before the initial prompt is ready.
Maybe it's ok to create a new GUID whenever the omp Lua script gets reloaded in the session? That won't be often, but it can definitely happen multiple times in a single cmd.exe process session. If omp can tolerate the POSH_PID periodically changing in a given session, then that would greatly simplify the problem.
Yes, it's just okay. Normally, the initialization Lua script will not be loaded more than once.
lewis-yeung
commented
1 day ago @JanDeDobbeleer As there is indeed an issue of outdated cache being reused unexpectedly, I have made a PR to use UUIDs for all supported shells.
Code of Conduct
What happened?
Hi @JanDeDobbeleer, I've reproduced this problem on both oh-my-posh v23.14.1 and v13.1.2, so this problem has been around for a long time.
My cmd process id is 42508. Running
oh-my-posh print primaryshould use the default theme. But instead it uses a seemingly random theme. But only in one cmd.exe process -- in others it works as expected.I believe I've tracked it down to a problem in how
omp.cacheis managed:My
omp.cachefile has 153 entries. There's an entry for 42508, but it's from May 2022.The timestamp is from May 2022:
The cached environment in the 42508 entry has
\"POSH_THEME\":\"c:\\\\wbin\\\\oh-my-posh\\\\lambdageneration.omp.json\", and the omp program interprets that as meaning it should that theme for all cmd.exe processes that ever get assigned a process id 42508 by the OS.At least on Windows, process id numbers are only unique at a given moment in time. 42508 could be the id of a cmd.exe, but once the cmd.exe process ends then the number can get assigned to any other new process (which may or may not be another cmd.exe process).
I don't know enough about how the
omp.cacheis used, so I don't know specifically what to suggest for how to solve the issue. I can help brainstorm solutions, if I know more about how it's used.I assume the omp program has some upper bound for the number of cache entries it will keep, and presumably it ages out old entries.
Theme
That's the problem -- it's picking a theme by mistake from an old cache entry from May 2022. 🙃
What OS are you seeing the problem on?
Windows
Which shell are you using?
cmd
Log output