From 69d58e0de823af68e9539285fca4996b5dedff8d Mon Sep 17 00:00:00 2001
From: Kristoffer Carlsson <kcarlsson89@gmail.com>
Date: Mon, 8 Jan 2024 14:17:47 +0100
Subject: [PATCH] use a Dict instead of an IdDict for caching of the `cwstring`
 for Windows env variables (#52758)

Should fix https://github.com/JuliaLang/julia/issues/52711.

My analysis of the invalidation is as follows:

We added code to cache the conversion to `cwstring` in env handling on
Windows (https://github.com/JuliaLang/julia/pull/51371):

```julia
const env_dict = IdDict{String, Vector{UInt16}}()

function memoized_env_lookup(str::AbstractString)
    ...
    env_dict[str] = cwstring(str)
    ...
end

function access_env(onError::Function, str::AbstractString)
    var = memoized_env_lookup(str)
    ...
end
```

Since `IdDict` has `@nospecialize` on `setindex!` we compile this
method:

```julia
setindex!(::IdDict{String, Vector{UInt16}}, ::Any, ::Any)
```

which has an edge to:

```julia
convert(Type{Vector{Int64}}, Any})
```

But then StaticArrays comes along and adds a method

```julia
convert(::Type{Array{T, N}}, ::StaticArray)
```

which invalidates the `setindex!` (due to the edge to `convert`) which
invalidates the whole env handling on Windows which causes 4k other
methods downstream to be invalidated, in particular, the artifact string
macro which causes a significant delay in the next jll package you load
after loading StaticArrays.

There should be no performance penalty to this since strings already
does a hash for their `objectid`.

(cherry picked from commit b7c24ed67bc42d51ee33da5ed5a97361c49313b8)
---
 base/env.jl | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/base/env.jl b/base/env.jl
index 27594acff6b7f..1bebfd2623e61 100644
--- a/base/env.jl
+++ b/base/env.jl
@@ -3,7 +3,7 @@
 if Sys.iswindows()
     const ERROR_ENVVAR_NOT_FOUND = UInt32(203)
 
-    const env_dict = IdDict{String, Vector{Cwchar_t}}()
+    const env_dict = Dict{String, Vector{Cwchar_t}}()
     const env_lock = ReentrantLock()
 
     function memoized_env_lookup(str::AbstractString)
@@ -11,13 +11,14 @@ if Sys.iswindows()
         # incurred allocations because we had to convert a String to a Vector{Cwchar_t} each time
         # an environment variable was looked up. This function memoizes that lookup process, storing
         # the String => Vector{Cwchar_t} pairs in env_dict
-        var = get(env_dict, str, nothing)
-        if isnothing(var)
-            var = @lock env_lock begin
-                env_dict[str] = cwstring(str)
+        @lock env_lock begin
+            var = get(env_dict, str, nothing)
+            if isnothing(var)
+                var = cwstring(str)
+                env_dict[str] = var
             end
+            return var
         end
-        var
     end
 
     _getenvlen(var::Vector{UInt16}) = ccall(:GetEnvironmentVariableW,stdcall,UInt32,(Ptr{UInt16},Ptr{UInt16},UInt32),var,C_NULL,0)