Add Lockable to Base, to bundle a lock with its resource (#52898)

I am not sure about a `lock(f, ::Lockable)` method: it is nice because
it unpacks the value for you, but it is weird because it unpacks the
value for you. For a `Lockable`, `f` must accept one argument, whereas
for a `Lock`, `f` must be 0-arg. A `Lockable` is not `<:AbstractLock`
here, so maybe this is allowed, but if we deleted this `lock` method, we
could inherit from `AbstractLock` and just use the generic one
(requiring folks to unpack the value within the locked region
themselves, as is the case for `@lock`). I think it is preferred these
days to use `@lock` anyway, so having the `lock(f, ::Lockable)` method
may be of limited value anyway.

I searched Base and came up with two places that could currently use
this internally, `TEMP_CLEANUP` and `env_dict`. We didn't add them both
as usages yet, to avoid external breakage from delaying this PR.

Similarly, this is not exported yet, to avoid breakage with older
releases of ConcurrentUtilities.jl.

redo of #34400
First commit copied from
https://github.com/JuliaServices/ConcurrentUtilities.jl/blob/main/src/lockable.jl,
Closes #52897 

Co-authored-by: Jacob Quinn <[email protected]>
Co-authored-by: Dilum Aluthge <[email protected]>

NEWS.md

@@ -73,6 +73,7 @@ Multi-threading changes
 -----------------------
 
 * `Threads.@threads` now supports the `:greedy` scheduler, intended for non-uniform workloads ([#52096]).
+* A new exported struct `Lockable{T, L<:AbstractLock}` makes it easy to bundle a resource and its lock together ([#52898]).
 
 Build system changes
 --------------------

base/env.jl

@@ -3,19 +3,18 @@
 if Sys.iswindows()
     const ERROR_ENVVAR_NOT_FOUND = UInt32(203)
 
-    const env_dict = Dict{String, Vector{Cwchar_t}}()
-    const env_lock = ReentrantLock()
+    const env_dict = Lockable(Dict{String, Vector{Cwchar_t}}())
 
     function memoized_env_lookup(str::AbstractString)
         # Windows environment variables have a different format from Linux / MacOS, and previously
         # 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
-        @lock env_lock begin
-            var = get(env_dict, str, nothing)
+        @lock env_dict begin
+            var = get(env_dict[], str, nothing)
             if isnothing(var)
                 var = cwstring(str)
-                env_dict[str] = var
+                env_dict[][str] = var
             end
             return var
         end

base/lock.jl

@@ -294,6 +294,63 @@ macro lock_nofail(l, expr)
     end
 end
 
+"""
+  Lockable(value, lock = ReentrantLock())
+
+Creates a `Lockable` object that wraps `value` and
+associates it with the provided `lock`. This object
+supports [`@lock`](@ref), [`lock`](@ref), [`trylock`](@ref),
+[`unlock`](@ref). To access the value, index the lockable object while
+holding the lock.
+
+!!! compat "Julia 1.11"
+    Requires at least Julia 1.11.
+
+## Example
+
+```jldoctest
+julia> locked_list = Base.Lockable(Int[]);
+
+julia> @lock(locked_list, push!(locked_list[], 1)) # must hold the lock to access the value
+1-element Vector{Int64}:
+ 1
+
+julia> lock(summary, locked_list)
+"1-element Vector{Int64}"
+```
+"""
+struct Lockable{T, L <: AbstractLock}
+    value::T
+    lock::L
+end
+
+Lockable(value) = Lockable(value, ReentrantLock())
+getindex(l::Lockable) = (assert_havelock(l.lock); l.value)
+
+"""
+  lock(f::Function, l::Lockable)
+
+Acquire the lock associated with `l`, execute `f` with the lock held,
+and release the lock when `f` returns. `f` will receive one positional
+argument: the value wrapped by `l`. If the lock is already locked by a
+different task/thread, wait for it to become available.
+When this function returns, the `lock` has been released, so the caller should
+not attempt to `unlock` it.
+
+!!! compat "Julia 1.11"
+    Requires at least Julia 1.11.
+"""
+function lock(f, l::Lockable)
+    lock(l.lock) do
+        f(l.value)
+    end
+end
+
+# implement the rest of the Lock interface on Lockable
+lock(l::Lockable) = lock(l.lock)
+trylock(l::Lockable) = trylock(l.lock)
+unlock(l::Lockable) = unlock(l.lock)
+
 @eval Threads begin
     """
         Threads.Condition([lock])

doc/src/base/parallel.md

@@ -51,6 +51,7 @@ Base.trylock
 Base.islocked
 Base.ReentrantLock
 Base.@lock
+Base.Lockable
 ```
 
 ## Channels

test/misc.jl

@@ -129,6 +129,36 @@ let l = ReentrantLock()
     @test_throws ErrorException unlock(l)
 end
 
+# Lockable{T, L<:AbstractLock}
+using Base: Lockable
+let
+    @test_broken Base.isexported(Base, :Lockable)
+    lockable = Lockable(Dict("foo" => "hello"), ReentrantLock())
+    # note field access is non-public
+    @test lockable.value["foo"] == "hello"
+    @test @lock(lockable, lockable[]["foo"]) == "hello"
+    lock(lockable) do d
+        @test d["foo"] == "hello"
+    end
+    lock(lockable) do d
+        d["foo"] = "goodbye"
+    end
+    @test lockable.value["foo"] == "goodbye"
+    @lock lockable begin
+        @test lockable[]["foo"] == "goodbye"
+    end
+    l = trylock(lockable)
+    try
+        @test l
+    finally
+        unlock(lockable)
+    end
+    # Test 1-arg constructor
+    lockable2 = Lockable(Dict("foo" => "hello"))
+    @test lockable2.lock isa ReentrantLock
+    @test @lock(lockable2, lockable2[]["foo"]) == "hello"
+end
+
 for l in (Threads.SpinLock(), ReentrantLock())
     @test get_finalizers_inhibited() == 0
     @test lock(get_finalizers_inhibited, l) == 1