From c8517353f05f7be00de3cc15ce92a2bd8d01387a Mon Sep 17 00:00:00 2001
From: Tim Holy <tim.holy@gmail.com>
Date: Sun, 5 Dec 2021 08:43:35 -0600
Subject: [PATCH] Yield after `require`

Under specific circumstances (package callbacks that run under
`@async`), this can change the ordering of operations:
some may miss a `yield` window due to locking. This adds
a `yield` when the lock is freed.
---
 base/loading.jl |  4 ++--
 base/lock.jl    |  9 +++++++--
 test/loading.jl | 37 +++++++++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/base/loading.jl b/base/loading.jl
index e3352d0d0b4ae..160b865f8f246 100644
--- a/base/loading.jl
+++ b/base/loading.jl
@@ -1037,7 +1037,7 @@ function require(into::Module, mod::Symbol)
     finally
         LOADING_CACHE[] = nothing
     end
-    end
+    end #= @lock unlock_expr =# yield()
 end
 
 mutable struct PkgOrigin
@@ -1067,7 +1067,7 @@ function require(uuidkey::PkgId)
               module `$(uuidkey.name)`, check for typos in package module name")
     end
     return root_module(uuidkey)
-    end
+    end #= @lock unlock_expr =# yield()
 end
 
 const loaded_modules = Dict{PkgId,Module}()
diff --git a/base/lock.jl b/base/lock.jl
index f3fdd8822dae2..b07ac25098014 100644
--- a/base/lock.jl
+++ b/base/lock.jl
@@ -198,7 +198,7 @@ function trylock(f, l::AbstractLock)
 end
 
 """
-    @lock l expr
+    @lock l expr [unlock_expr=nothing]
 
 Macro version of `lock(f, l::AbstractLock)` but with `expr` instead of `f` function.
 Expands to:
@@ -208,12 +208,16 @@ try
     expr
 finally
     unlock(l)
+    unlock_expr
 end
 ```
 This is similar to using [`lock`](@ref) with a `do` block, but avoids creating a closure
 and thus can improve the performance.
+
+!!! compat "Julia 1.8"
+    `unlock_expr` requires at least Julia 1.8
 """
-macro lock(l, expr)
+macro lock(l, expr, unlock_expr=nothing)
     quote
         temp = $(esc(l))
         lock(temp)
@@ -221,6 +225,7 @@ macro lock(l, expr)
             $(esc(expr))
         finally
             unlock(temp)
+            $(esc(unlock_expr))
         end
     end
 end
diff --git a/test/loading.jl b/test/loading.jl
index 89d61207e33d9..f1cdd850afc64 100644
--- a/test/loading.jl
+++ b/test/loading.jl
@@ -766,3 +766,40 @@ end
         end
     end
 end
+
+@testset "package callbacks that acquire `require_lock` under `@async`" begin
+    # https://github.com/JuliaLang/julia/pull/41602#issuecomment-986115131
+    mktempdir() do tmp
+        push!(LOAD_PATH, tmp)
+        write(joinpath(tmp, "Outer41602.jl"), """
+            module Outer41602
+            using Inner41602
+
+            const callback_timing = []
+            function __init__()
+                push!(Base.package_callbacks, id -> @async push!(callback_timing, (Base.root_module(id), time()-Inner41602.callback_timing[1])))
+            end
+
+            end
+        """)
+        write(joinpath(tmp, "Inner41602.jl"), """
+            module Inner41602
+
+            const callback_timing = []
+            function __init__()
+                push!(callback_timing, time())
+                push!(Base.package_callbacks, id -> @async push!(callback_timing, (Base.root_module(id), time()-callback_timing[1])))
+            end
+
+            end
+        """)
+        @eval using Outer41602
+        cti, cto = copy(Outer41602.Inner41602.callback_timing), copy(Outer41602.callback_timing)
+        @test length(cti) == 3
+        @test length(cto) == 1
+        @test cti[2][1] == Outer41602.Inner41602
+        @test cti[3][1] == Outer41602
+        @test cti[end][end] < cto[end][end]
+        pop!(LOAD_PATH)
+    end
+end