From f8dddedf8ff1ec4cc29e58d2a7b03fb87ab44b69 Mon Sep 17 00:00:00 2001
From: Ian Butterworth <i.r.butterworth@gmail.com>
Date: Mon, 29 Jan 2024 14:51:11 -0500
Subject: [PATCH 1/2] warn if cycle detected in extension precompilation

---
 base/loading.jl                         | 10 ++++++--
 test/extensions/circular/A/Project.toml | 14 +++++++++++
 test/extensions/circular/A/ext/CExt.jl  |  3 +++
 test/extensions/circular/A/ext/DExt.jl  |  3 +++
 test/extensions/circular/A/src/A.jl     |  3 +++
 test/extensions/circular/B/Project.toml |  7 ++++++
 test/extensions/circular/B/src/B.jl     |  3 +++
 test/extensions/circular/C/Project.toml |  3 +++
 test/extensions/circular/C/src/C.jl     |  3 +++
 test/extensions/circular/D/Project.toml |  3 +++
 test/extensions/circular/D/src/D.jl     |  3 +++
 test/extensions/circular/Manifest.toml  | 32 +++++++++++++++++++++++++
 test/extensions/circular/Project.toml   |  5 ++++
 test/loading.jl                         | 13 ++++++++++
 14 files changed, 103 insertions(+), 2 deletions(-)
 create mode 100644 test/extensions/circular/A/Project.toml
 create mode 100644 test/extensions/circular/A/ext/CExt.jl
 create mode 100644 test/extensions/circular/A/ext/DExt.jl
 create mode 100644 test/extensions/circular/A/src/A.jl
 create mode 100644 test/extensions/circular/B/Project.toml
 create mode 100644 test/extensions/circular/B/src/B.jl
 create mode 100644 test/extensions/circular/C/Project.toml
 create mode 100644 test/extensions/circular/C/src/C.jl
 create mode 100644 test/extensions/circular/D/Project.toml
 create mode 100644 test/extensions/circular/D/src/D.jl
 create mode 100644 test/extensions/circular/Manifest.toml
 create mode 100644 test/extensions/circular/Project.toml

diff --git a/base/loading.jl b/base/loading.jl
index 882ae3a68a836..e689532a000a7 100644
--- a/base/loading.jl
+++ b/base/loading.jl
@@ -1484,6 +1484,12 @@ function run_extension_callbacks(pkgid::PkgId)
     extids = pop!(EXT_DORMITORY, pkgid, nothing)
     extids === nothing && return
     for extid in extids
+        if in(extid.id, precompilation_stack)
+            @warn """
+            Dependency cycle detected in extension precompilation: $(precompilation_stack_list()) > $(extid.id.name)
+            Loading $(extid.id.name) here will likely fail.
+            """
+        end
         if extid.ntriggers > 0
             # indicate pkgid is loaded
             extid.ntriggers -= 1
@@ -2842,14 +2848,14 @@ function create_expr_cache(pkg::PkgId, input::String, output::String, output_o::
 end
 
 const precompilation_stack = Vector{PkgId}()
+precompilation_stack_list() = join(map(p->p.name, precompilation_stack), " > ")
 # Helpful for debugging when precompilation is unexpectedly nested.
 # Enable with `JULIA_DEBUG=nested_precomp`. Note that it expected to be nested in classical code-load precompilation
 # TODO: Add detection if extension precompilation is nested and error / return early?
 function track_nested_precomp(pkgs::Vector{PkgId})
     append!(precompilation_stack, pkgs)
     if length(precompilation_stack) > 1
-        list() = join(map(p->p.name, precompilation_stack), " > ")
-        @debug "Nested precompilation: $(list())" _group=:nested_precomp
+        @debug "Nested precompilation: $(precompilation_stack_list())" _group=:nested_precomp
     end
 end
 
diff --git a/test/extensions/circular/A/Project.toml b/test/extensions/circular/A/Project.toml
new file mode 100644
index 0000000000000..9b55fcbf05001
--- /dev/null
+++ b/test/extensions/circular/A/Project.toml
@@ -0,0 +1,14 @@
+name = "A"
+uuid = "c8938445-7efa-411c-ba8f-721576f9f47a"
+version = "0.1.0"
+
+[deps]
+B = "ef00b25e-dd85-47e0-9cd9-ed3c1ff51032"
+
+[weakdeps]
+C = "d95b043b-fbe6-48a2-93d2-75fba1b51518"
+D = "64e66ba5-e914-4adb-a5f6-036ef0c801f3"
+
+[extensions]
+CExt = "C"
+DExt = "D"
diff --git a/test/extensions/circular/A/ext/CExt.jl b/test/extensions/circular/A/ext/CExt.jl
new file mode 100644
index 0000000000000..33b4a5fc7cd3c
--- /dev/null
+++ b/test/extensions/circular/A/ext/CExt.jl
@@ -0,0 +1,3 @@
+module CExt
+
+end # module
diff --git a/test/extensions/circular/A/ext/DExt.jl b/test/extensions/circular/A/ext/DExt.jl
new file mode 100644
index 0000000000000..915dfc89196f2
--- /dev/null
+++ b/test/extensions/circular/A/ext/DExt.jl
@@ -0,0 +1,3 @@
+module DExt
+
+end # module
diff --git a/test/extensions/circular/A/src/A.jl b/test/extensions/circular/A/src/A.jl
new file mode 100644
index 0000000000000..b555e27e24287
--- /dev/null
+++ b/test/extensions/circular/A/src/A.jl
@@ -0,0 +1,3 @@
+module A
+
+end # module
diff --git a/test/extensions/circular/B/Project.toml b/test/extensions/circular/B/Project.toml
new file mode 100644
index 0000000000000..21205c7c345a5
--- /dev/null
+++ b/test/extensions/circular/B/Project.toml
@@ -0,0 +1,7 @@
+name = "B"
+uuid = "ef00b25e-dd85-47e0-9cd9-ed3c1ff51032"
+version = "0.1.0"
+
+[deps]
+C = "d95b043b-fbe6-48a2-93d2-75fba1b51518"
+D = "64e66ba5-e914-4adb-a5f6-036ef0c801f3"
diff --git a/test/extensions/circular/B/src/B.jl b/test/extensions/circular/B/src/B.jl
new file mode 100644
index 0000000000000..d5bdf6392c9f4
--- /dev/null
+++ b/test/extensions/circular/B/src/B.jl
@@ -0,0 +1,3 @@
+module B
+
+end # module
diff --git a/test/extensions/circular/C/Project.toml b/test/extensions/circular/C/Project.toml
new file mode 100644
index 0000000000000..a913862e5a36f
--- /dev/null
+++ b/test/extensions/circular/C/Project.toml
@@ -0,0 +1,3 @@
+name = "C"
+uuid = "d95b043b-fbe6-48a2-93d2-75fba1b51518"
+version = "0.1.0"
diff --git a/test/extensions/circular/C/src/C.jl b/test/extensions/circular/C/src/C.jl
new file mode 100644
index 0000000000000..489fa1e07ea91
--- /dev/null
+++ b/test/extensions/circular/C/src/C.jl
@@ -0,0 +1,3 @@
+module C
+
+end # module
diff --git a/test/extensions/circular/D/Project.toml b/test/extensions/circular/D/Project.toml
new file mode 100644
index 0000000000000..0e7bda4162265
--- /dev/null
+++ b/test/extensions/circular/D/Project.toml
@@ -0,0 +1,3 @@
+name = "D"
+uuid = "64e66ba5-e914-4adb-a5f6-036ef0c801f3"
+version = "0.1.0"
diff --git a/test/extensions/circular/D/src/D.jl b/test/extensions/circular/D/src/D.jl
new file mode 100644
index 0000000000000..e87de9cb2024a
--- /dev/null
+++ b/test/extensions/circular/D/src/D.jl
@@ -0,0 +1,3 @@
+module D
+
+end # module
diff --git a/test/extensions/circular/Manifest.toml b/test/extensions/circular/Manifest.toml
new file mode 100644
index 0000000000000..cd5f52f1de4ec
--- /dev/null
+++ b/test/extensions/circular/Manifest.toml
@@ -0,0 +1,32 @@
+# This file is machine-generated - editing it directly is not advised
+
+julia_version = "1.11.0-DEV"
+manifest_format = "2.0"
+project_hash = "6c189c4d9e1e9194c4a4d7627740c379ecff4942"
+
+[[deps.A]]
+deps = ["B"]
+path = "./A"
+uuid = "c8938445-7efa-411c-ba8f-721576f9f47a"
+version = "0.1.0"
+weakdeps = ["C", "D"]
+
+    [deps.A.extensions]
+    CExt = "C"
+    DExt = "D"
+
+[[deps.B]]
+deps = ["C", "D"]
+path = "./B"
+uuid = "ef00b25e-dd85-47e0-9cd9-ed3c1ff51032"
+version = "0.1.0"
+
+[[deps.C]]
+path = "./C"
+uuid = "d95b043b-fbe6-48a2-93d2-75fba1b51518"
+version = "0.1.0"
+
+[[deps.D]]
+path = "./D"
+uuid = "64e66ba5-e914-4adb-a5f6-036ef0c801f3"
+version = "0.1.0"
diff --git a/test/extensions/circular/Project.toml b/test/extensions/circular/Project.toml
new file mode 100644
index 0000000000000..beed7a9072ad4
--- /dev/null
+++ b/test/extensions/circular/Project.toml
@@ -0,0 +1,5 @@
+[deps]
+A = "c8938445-7efa-411c-ba8f-721576f9f47a"
+B = "ef00b25e-dd85-47e0-9cd9-ed3c1ff51032"
+C = "d95b043b-fbe6-48a2-93d2-75fba1b51518"
+D = "64e66ba5-e914-4adb-a5f6-036ef0c801f3"
diff --git a/test/loading.jl b/test/loading.jl
index 94f27ecf888bb..ee34f0f0c917c 100644
--- a/test/loading.jl
+++ b/test/loading.jl
@@ -1143,6 +1143,19 @@ end
         finally
             copy!(LOAD_PATH, old_load_path)
         end
+
+        # issue 53081
+        @testset "avoid circular precompilation deadlock through extensions" begin
+            testenv = joinpath(@__DIR__, "extensions", "circular")
+            # TODO: avoid the error? https://github.com/JuliaLang/julia/pull/53112
+            # @test_warn (r"Dependency cycle detected in extension precompilation", r"Error during loading of extension") begin
+                run(ignorestatus(addenv(
+                    `$(Base.julia_cmd()) --startup-file=no --project=$testenv -e 'using A'`,
+                    "JULIA_TESTS" => nothing, # we want precompilepkgs to print for this, which it doesn't if this is set
+                    "JULIA_DEPOT_PATH" => depot_path
+                )))
+            # end
+        end
     finally
         try
             rm(depot_path, force=true, recursive=true)

From a3803379605b8bb2547b08bfcf068b2f458df91e Mon Sep 17 00:00:00 2001
From: Ian Butterworth <i.r.butterworth@gmail.com>
Date: Thu, 4 Apr 2024 20:44:23 -0400
Subject: [PATCH 2/2] fix

---
 base/loading.jl | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/base/loading.jl b/base/loading.jl
index e689532a000a7..25b46dbd85fd1 100644
--- a/base/loading.jl
+++ b/base/loading.jl
@@ -1484,12 +1484,6 @@ function run_extension_callbacks(pkgid::PkgId)
     extids = pop!(EXT_DORMITORY, pkgid, nothing)
     extids === nothing && return
     for extid in extids
-        if in(extid.id, precompilation_stack)
-            @warn """
-            Dependency cycle detected in extension precompilation: $(precompilation_stack_list()) > $(extid.id.name)
-            Loading $(extid.id.name) here will likely fail.
-            """
-        end
         if extid.ntriggers > 0
             # indicate pkgid is loaded
             extid.ntriggers -= 1
@@ -1504,6 +1498,12 @@ function run_extension_callbacks(pkgid::PkgId)
         if extid.ntriggers == 0
             # actually load extid, now that all dependencies are met,
             # and record the result
+            if in(extid.id, precompilation_stack)
+                @warn """
+                Dependency cycle detected in extension precompilation: $(precompilation_stack_list()) > $(extid.id.name)
+                Loading $(extid.id.name) here will likely fail.
+                """
+            end
             succeeded = succeeded && run_extension_callbacks(extid)
             succeeded || push!(EXT_DORMITORY_FAILED, extid)
         end