From 57f02bfaeb5dca0d7774ccabdf92ac57baccff0c Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Mon, 12 Feb 2024 02:33:29 +0100 Subject: [PATCH] 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 https://github.com/JuliaLang/julia/pull/34400 First commit copied from https://github.com/JuliaServices/ConcurrentUtilities.jl/blob/main/src/lockable.jl, Closes #52897 Co-authored-by: Jacob Quinn Co-authored-by: Dilum Aluthge --- NEWS.md | 1 + base/env.jl | 9 +++---- base/lock.jl | 57 ++++++++++++++++++++++++++++++++++++++++ doc/src/base/parallel.md | 1 + test/misc.jl | 30 +++++++++++++++++++++ 5 files changed, 93 insertions(+), 5 deletions(-) diff --git a/NEWS.md b/NEWS.md index 3c83e995c4de2..bf4e72afc95d0 100644 --- a/NEWS.md +++ b/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 -------------------- diff --git a/base/env.jl b/base/env.jl index 3092efbe372e0..2928a8bdfede2 100644 --- a/base/env.jl +++ b/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 diff --git a/base/lock.jl b/base/lock.jl index eb76a3b30e6a4..7cbb023a78ee4 100644 --- a/base/lock.jl +++ b/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]) diff --git a/doc/src/base/parallel.md b/doc/src/base/parallel.md index c3106b8caf8c7..5798da54a5cde 100644 --- a/doc/src/base/parallel.md +++ b/doc/src/base/parallel.md @@ -51,6 +51,7 @@ Base.trylock Base.islocked Base.ReentrantLock Base.@lock +Base.Lockable ``` ## Channels diff --git a/test/misc.jl b/test/misc.jl index d2a8fabd119c5..6597ecf8a8428 100644 --- a/test/misc.jl +++ b/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