Finalizers take function as first argument (closes #16307) (#24605)

make finalizers take function as first argument

Author: oxinabox

NEWS.md

@@ -275,7 +275,13 @@ This section lists changes that do not have deprecation warnings.
   * The return type of `reinterpret` has changed to `ReinterpretArray`. `reinterpret` on sparse
     arrays has been discontinued.
 
-  * `Base.find_in_path` is now `Base.find_package` or `Base.find_source_file` ([#24320])
+  * `Base.find_in_path` is now `Base.find_package` or `Base.find_source_file` ([#24320]).
+
+  * `finalizer` now takes functions or pointers as its first argument, and the object being
+    finalized as its second (rather than the reverse). For the majority of use cases
+    deprecation warnings will be triggered. However, deprecation warnings will not trigger where
+    (1) the callable argument is not a subtype of `Function`; or (2) both arguments are
+    `Function`s or `Ptr{Void}`s ([#24605]).
 
   * The `kill` function now throws errors on user error (e.g. on permission
     errors), but returns successfully if the process had previously exited.

base/deprecated.jl

@@ -2200,6 +2200,17 @@ end
     @deprecate chol!(x::Number, uplo) chol(x) false
 end
 
+
+# issue #16307
+@deprecate finalizer(o, f::Function) finalizer(f, o)
+# This misses other callables but they are very rare in the wild
+@deprecate finalizer(o, f::Ptr{Void}) finalizer(f, o)
+
+# Avoid ambiguity, can remove when deprecations are removed:
+# This is almost certainly going to be a silent failure for code that is not updated.
+finalizer(f::Ptr{Void}, o::Ptr{Void}) = invoke(finalizer, Tuple{Ptr{Void}, Any}, f, o)
+finalizer(f::Ptr{Void}, o::Function) = invoke(finalizer, Tuple{Ptr{Void}, Any}, f, o)
+
 # END 0.7 deprecations
 
 # BEGIN 1.0 deprecations

base/distributed/cluster.jl

@@ -518,7 +518,11 @@ function create_worker(manager, wconfig)
 
     w = Worker(w.id, r_s, w_s, manager; config=wconfig)
     # install a finalizer to perform cleanup if necessary
-    finalizer(w, (w)->if myid() == 1 manage(w.manager, w.id, w.config, :finalize) end)
+    finalizer(w) do w
+        if myid() == 1
+            manage(w.manager, w.id, w.config, :finalize)
+        end
+    end
 
     # set when the new worker has finshed connections with all other workers
     ntfy_oid = RRID()

base/distributed/clusterserialize.jl

@@ -74,7 +74,9 @@ function serialize(s::ClusterSerializer, t::TypeName)
         serialize_typename(s, t)
         s.anonfunc_id = prev
         push!(s.tn_obj_sent, identifier)
-        finalizer(t, x->cleanup_tname_glbs(s, identifier))
+        finalizer(t) do x
+            cleanup_tname_glbs(s, identifier)
+        end
     end
 
     # Send global refs if required.
@@ -133,7 +135,9 @@ function serialize_global_from_main(s::ClusterSerializer, sym)
     elseif !haskey(s.glbs_sent, oid)
         # set up a finalizer the first time this object is sent
         try
-            finalizer(v, x->delete_global_tracker(s,x))
+            finalizer(v) do x
+                delete_global_tracker(s,x)
+            end
         catch ex
             # Do not track objects that cannot be finalized.
             if isa(ex, ErrorException)

base/distributed/remotecall.jl

@@ -52,13 +52,13 @@ function test_existing_ref(r::AbstractRemoteRef)
     end
 
     client_refs[r] = nothing
-    finalizer(r, finalize_ref)
+    finalizer(finalize_ref, r)
     return r
 end
 
 function finalize_ref(r::AbstractRemoteRef)
     if r.where > 0 # Handle the case of the finalizer having been called manually
-        islocked(client_refs) && return finalizer(r, finalize_ref) # delay finalizer for later, when it's not already locked
+        islocked(client_refs) && return finalizer(finalize_ref, r) # delay finalizer for later, when it's not already locked
         delete!(client_refs, r)
         if isa(r, RemoteChannel)
             send_del_client(r)

base/distributed/workerpool.jl

@@ -223,7 +223,7 @@ mutable struct CachingPool <: AbstractWorkerPool
 
     function CachingPool()
         wp = new(Channel{Int}(typemax(Int)), Set{Int}(), Dict{Int, Function}())
-        finalizer(wp, clear!)
+        finalizer(clear!, wp)
         wp
     end
 end

base/event.jl

@@ -303,7 +303,7 @@ mutable struct AsyncCondition
     function AsyncCondition()
         this = new(Libc.malloc(_sizeof_uv_async), Condition(), true)
         associate_julia_struct(this.handle, this)
-        finalizer(this, uvfinalize)
+        finalizer(uvfinalize, this)
         err = ccall(:uv_async_init, Cint, (Ptr{Void}, Ptr{Void}, Ptr{Void}),
             eventloop(), this, uv_jl_asynccb::Ptr{Void})
         if err != 0
@@ -372,7 +372,7 @@ mutable struct Timer
         end
 
         associate_julia_struct(this.handle, this)
-        finalizer(this, uvfinalize)
+        finalizer(uvfinalize, this)
 
         ccall(:uv_update_time, Void, (Ptr{Void},), eventloop())
         ccall(:uv_timer_start,  Cint,  (Ptr{Void}, Ptr{Void}, UInt64, UInt64),

base/gcutils.jl

@@ -5,21 +5,21 @@
 ==(w, v::WeakRef) = isequal(w, v.value)
 
 """
-    finalizer(x, f)
+    finalizer(f, x)
 
 Register a function `f(x)` to be called when there are no program-accessible references to
 `x`. The type of `x` must be a `mutable struct`, otherwise the behavior of this function is
 unpredictable.
 """
-function finalizer(@nospecialize(o), @nospecialize(f))
+function finalizer(@nospecialize(f), @nospecialize(o))
     if isimmutable(o)
         error("objects of type ", typeof(o), " cannot be finalized")
     end
     ccall(:jl_gc_add_finalizer_th, Void, (Ptr{Void}, Any, Any),
           Core.getptls(), o, f)
 end
 
-function finalizer(o::T, f::Ptr{Void}) where T
+function finalizer(f::Ptr{Void}, o::T) where T
     @_inline_meta
     if isimmutable(T)
         error("objects of type ", T, " cannot be finalized")

base/gmp.jl

@@ -55,7 +55,7 @@ mutable struct BigInt <: Signed
     function BigInt()
         b = new(zero(Cint), zero(Cint), C_NULL)
         MPZ.init!(b)
-        finalizer(b, cglobal((:__gmpz_clear, :libgmp)))
+        finalizer(cglobal((:__gmpz_clear, :libgmp)), b)
         return b
     end
 end

base/iostream.jl

@@ -18,7 +18,7 @@ function IOStream(name::AbstractString, finalize::Bool)
     buf = zeros(UInt8,sizeof_ios_t)
     x = IOStream(name, buf)
     if finalize
-        finalizer(x, close)
+        finalizer(close, x)
     end
     return x
 end

base/libgit2/gitcredential.jl

@@ -19,7 +19,7 @@ mutable struct GitCredential
             username::Nullable{<:AbstractString},
             password::Nullable{<:AbstractString})
         c = new(protocol, host, path, username, password, true)
-        finalizer(c, securezero!)
+        finalizer(securezero!, c)
         return c
     end
 end

base/libgit2/types.jl

@@ -944,7 +944,7 @@ for (typ, owntyp, sup, cname) in [
                 obj = new(ptr)
                 if fin
                     Threads.atomic_add!(REFCOUNT, UInt(1))
-                    finalizer(obj, Base.close)
+                    finalizer(Base.close, obj)
                 end
                 return obj
             end
@@ -958,7 +958,7 @@ for (typ, owntyp, sup, cname) in [
                 obj = new(owner, ptr)
                 if fin
                     Threads.atomic_add!(REFCOUNT, UInt(1))
-                    finalizer(obj, Base.close)
+                    finalizer(Base.close, obj)
                 end
                 return obj
             end
@@ -1000,7 +1000,7 @@ mutable struct GitSignature <: AbstractGitObject
     function GitSignature(ptr::Ptr{SignatureStruct})
         @assert ptr != C_NULL
         obj = new(ptr)
-        finalizer(obj, Base.close)
+        finalizer(Base.close, obj)
         return obj
     end
 end
@@ -1144,7 +1144,7 @@ mutable struct UserPasswordCredentials <: AbstractCredentials
     pass::String
     function UserPasswordCredentials(user::AbstractString="", pass::AbstractString="")
         c = new(user, pass)
-        finalizer(c, securezero!)
+        finalizer(securezero!, c)
         return c
     end
 
@@ -1182,7 +1182,7 @@ mutable struct SSHCredentials <: AbstractCredentials
     function SSHCredentials(user::AbstractString="", pass::AbstractString="",
                             prvkey::AbstractString="", pubkey::AbstractString="")
         c = new(user, pass, prvkey, pubkey)
-        finalizer(c, securezero!)
+        finalizer(securezero!, c)
         return c
     end
 

base/locks.jl

@@ -185,7 +185,7 @@ mutable struct Mutex <: AbstractLock
     function Mutex()
         m = new(zero(Int16), Libc.malloc(UV_MUTEX_SIZE))
         ccall(:uv_mutex_init, Void, (Ptr{Void},), m.handle)
-        finalizer(m, _uv_hook_close)
+        finalizer(_uv_hook_close, m)
         return m
     end
 end

base/mpfr.jl

@@ -63,7 +63,7 @@ mutable struct BigFloat <: AbstractFloat
         prec = precision(BigFloat)
         z = new(zero(Clong), zero(Cint), zero(Clong), C_NULL)
         ccall((:mpfr_init2,:libmpfr), Void, (Ref{BigFloat}, Clong), z, prec)
-        finalizer(z, cglobal((:mpfr_clear, :libmpfr)))
+        finalizer(cglobal((:mpfr_clear, :libmpfr)), z)
         return z
     end
 
@@ -969,7 +969,7 @@ function Base.deepcopy_internal(x::BigFloat, stackdict::ObjectIdDict)
     prec = precision(x)
     y = BigFloat(zero(Clong), zero(Cint), zero(Clong), C_NULL)
     ccall((:mpfr_init2,:libmpfr), Void, (Ref{BigFloat}, Clong), y, prec)
-    finalizer(y, cglobal((:mpfr_clear, :libmpfr)))
+    finalizer(cglobal((:mpfr_clear, :libmpfr)), y)
     ccall((:mpfr_set, :libmpfr), Int32, (Ref{BigFloat}, Ref{BigFloat}, Int32), y, x, ROUNDING_MODE[])
     stackdict[x] = y
     return y

base/precompile.jl

@@ -19,7 +19,7 @@ precompile(Tuple{typeof(Base.copy!), Array{String, 1}, Int64, Array{Any, 1}, Int
 precompile(Tuple{typeof(Base.empty!), Base.Dict{Int64, Union{Base.Distributed.Worker, Base.Distributed.LocalProcess}}})
 precompile(Tuple{typeof(Core.Inference.isbits), Base.Distributed.DefaultClusterManager})
 precompile(Tuple{typeof(Base.Distributed.init_worker), String, Base.Distributed.DefaultClusterManager})
-precompile(Tuple{typeof(Base.finalizer), Base.TCPServer, typeof(Base.uvfinalize)})
+precompile(Tuple{typeof(Base.finalizer), typeof(Base.uvfinalize), Base.TCPServer})
 precompile(Tuple{Type{Base.TCPServer}, Ptr{Void}, Int64})
 precompile(Tuple{typeof(Base.show), Base.IOContext{Base.GenericIOBuffer{Array{UInt8, 1}}}, Int32})
 precompile(Tuple{typeof(Base.print), Base.GenericIOBuffer{Array{UInt8, 1}}, Base.Libc.RawFD})
@@ -105,7 +105,7 @@ precompile(Tuple{typeof(Core.Inference.start), Core.Inference.Generator{Tuple{Tu
 precompile(Tuple{typeof(Core.Inference.done), Core.Inference.Generator{Tuple{Tuple{Base.DevNullStream, Base.DevNullStream, Base.DevNullStream}}, Type{QuoteNode}}, Int64})
 precompile(Tuple{typeof(Core.Inference.next), Core.Inference.Generator{Tuple{Tuple{Base.DevNullStream, Base.DevNullStream, Base.DevNullStream}}, Type{QuoteNode}}, Int64})
 precompile(Tuple{typeof(Core.Inference.isbits), Base.DevNullStream})
-precompile(Tuple{typeof(Base.finalizer), Base.Process, typeof(Base.uvfinalize)})
+precompile(Tuple{typeof(Base.finalizer), typeof(Base.uvfinalize), Base.Process})
 precompile(Tuple{typeof(Core.Inference.isbits), Tuple{Base.DevNullStream, Bool}})
 precompile(Tuple{Type{Ref{Base.Cstring}}, Array{String, 1}})
 precompile(Tuple{typeof(Core.Inference.eltype), Type{Array{String, 1}}})
@@ -1552,7 +1552,7 @@ precompile(Tuple{typeof(Base.rehash!), Base.Dict{WeakRef, Void}, Int64})
 precompile(Tuple{typeof(Base.ht_keyindex2), Base.Dict{WeakRef, Void}, WeakRef})
 precompile(Tuple{typeof(Base._setindex!), Base.Dict{WeakRef, Void}, Void, WeakRef, Int64})
 precompile(Tuple{typeof(Base.setindex!), Base.Dict{WeakRef, Void}, Void, WeakRef})
-precompile(Tuple{typeof(Base.finalizer), Base.Distributed.RemoteChannel{Base.Channel{Any}}, typeof(Base.Distributed.finalize_ref)})
+precompile(Tuple{typeof(Base.finalizer),  typeof(Base.Distributed.finalize_ref), Base.Distributed.RemoteChannel{Base.Channel{Any}}})
 precompile(Tuple{typeof(Base.Distributed.test_existing_ref), Base.Distributed.RemoteChannel{Base.Channel{Any}}})
 precompile(Tuple{Type{Base.Distributed.RemoteChannel{T} where T<:Base.AbstractChannel}, Int64})
 precompile(Tuple{Type{Base.Channel{Int64}}, Int64})
@@ -1714,7 +1714,7 @@ precompile(Tuple{typeof(Base.Filesystem.mkdir), String, UInt16})
 precompile(Tuple{typeof(Base.Filesystem.mkpath), String, UInt16})
 precompile(Tuple{typeof(Base.Filesystem.samefile), String, String})
 precompile(Tuple{typeof(Base.Filesystem.unlink), String})
-precompile(Tuple{typeof(Base.finalizer), Base.Distributed.Future, typeof(Base.Distributed.finalize_ref)})
+precompile(Tuple{typeof(Base.finalizer), typeof(Base.Distributed.finalize_ref), Base.Distributed.Future})
 precompile(Tuple{typeof(Base.find_all_in_cache_path), Symbol})
 precompile(Tuple{typeof(Base.find_package), String})
 precompile(Tuple{typeof(Base.find_source_file), String})

base/process.jl

@@ -328,7 +328,7 @@ mutable struct Process <: AbstractPipe
                    typemin(fieldtype(Process, :exitcode)),
                    typemin(fieldtype(Process, :termsignal)),
                    Condition(), Condition())
-        finalizer(this, uvfinalize)
+        finalizer(uvfinalize, this)
         return this
     end
 end

base/regex.jl

@@ -29,10 +29,10 @@ mutable struct Regex
         end
         re = compile(new(pattern, compile_options, match_options, C_NULL,
                          C_NULL, Csize_t[], C_NULL))
-        finalizer(re, re->begin
-                              re.regex == C_NULL || PCRE.free_re(re.regex)
-                              re.match_data == C_NULL || PCRE.free_match_data(re.match_data)
-                          end)
+        finalizer(re) do re
+            re.regex == C_NULL || PCRE.free_re(re.regex)
+            re.match_data == C_NULL || PCRE.free_match_data(re.match_data)
+        end
         re
     end
 end

base/repl/REPL.jl

@@ -800,7 +800,9 @@ function setup_interface(
         try
             hist_path = find_hist_file()
             f = open(hist_path, true, true, true, false, false)
-            finalizer(replc, replc->close(f))
+            finalizer(replc) do replc
+                close(f)
+            end
             hist_from_file(hp, f, hist_path)
         catch e
             print_response(repl, e, catch_backtrace(), true, Base.have_color)

base/socket.jl

@@ -278,7 +278,7 @@ mutable struct TCPSocket <: LibuvStream
                 ReentrantLock(),
                 DEFAULT_READ_BUFFER_SZ)
         associate_julia_struct(tcp.handle, tcp)
-        finalizer(tcp, uvfinalize)
+        finalizer(uvfinalize, tcp)
         return tcp
     end
 end
@@ -308,7 +308,7 @@ mutable struct TCPServer <: LibuvServer
             Condition(),
             Condition())
         associate_julia_struct(tcp.handle, tcp)
-        finalizer(tcp, uvfinalize)
+        finalizer(uvfinalize, tcp)
         return tcp
     end
 end
@@ -370,7 +370,7 @@ mutable struct UDPSocket <: LibuvStream
             Condition(),
             Condition())
         associate_julia_struct(udp.handle, udp)
-        finalizer(udp, uvfinalize)
+        finalizer(uvfinalize, udp)
         return udp
     end
 end

base/sparse/cholmod.jl

@@ -204,7 +204,7 @@ mutable struct Dense{T<:VTypes} <: DenseMatrix{T}
                 "unknown reasons. Please submit a bug report."))
         end
         A = new(p)
-        finalizer(A, free!)
+        finalizer(free!, A)
         return A
     end
 end
@@ -256,7 +256,7 @@ mutable struct Sparse{Tv<:VTypes} <: AbstractSparseMatrix{Tv,SuiteSparse_long}
                 "unknown reasons. Please submit a bug report."))
         end
         A = new(p)
-        finalizer(A, free!)
+        finalizer(free!, A)
         return A
     end
 end
@@ -340,7 +340,7 @@ mutable struct Factor{Tv} <: Factorization{Tv}
         end
         F = new(p)
         if register_finalizer
-            finalizer(F, free!)
+            finalizer(free!, F)
         end
         return F
     end

base/sparse/umfpack.jl

@@ -143,7 +143,7 @@ function lufact(S::SparseMatrixCSC{<:UMFVTypes,<:UMFITypes})
                     zerobased ? copy(S.colptr) : decrement(S.colptr),
                     zerobased ? copy(S.rowval) : decrement(S.rowval),
                     copy(S.nzval))
-    finalizer(res, umfpack_free_symbolic)
+    finalizer(umfpack_free_symbolic, res)
     umfpack_numeric!(res)
 end
 lufact(A::SparseMatrixCSC{<:Union{Float16,Float32},Ti}) where {Ti<:UMFITypes} =

base/stream.jl

@@ -120,7 +120,7 @@ mutable struct PipeEndpoint <: LibuvStream
                 ReentrantLock(),
                 DEFAULT_READ_BUFFER_SZ)
         associate_julia_struct(handle, p)
-        finalizer(p, uvfinalize)
+        finalizer(uvfinalize, p)
         return p
     end
 end
@@ -136,7 +136,7 @@ mutable struct PipeServer <: LibuvServer
                 Condition(),
                 Condition())
         associate_julia_struct(p.handle, p)
-        finalizer(p, uvfinalize)
+        finalizer(uvfinalize, p)
         return p
     end
 end
@@ -169,7 +169,7 @@ mutable struct TTY <: LibuvStream
             nothing, ReentrantLock(),
             DEFAULT_READ_BUFFER_SZ)
         associate_julia_struct(handle, tty)
-        finalizer(tty, uvfinalize)
+        finalizer(uvfinalize, tty)
         @static if Sys.iswindows()
             tty.ispty = ccall(:jl_ispty, Cint, (Ptr{Void},), handle) != 0
         end