process: ensure uvfinalize and _uv_close_cb are synchronized (#44476)

There appeared to be a possibility they could race and the data field
might already be destroyed before we reached the close callback,
from looking at the state of the program when reproducing #44460.

This is because the uv_return_spawn set the handle to NULL, which later
can cause the uvfinalize to exit early (if the finalizer gets run on
another thread, since we have disabled finalizers on our thread). Then
the GC can reap the julia Process object prior to uv_close cleaning up
the object. We solve this by calling disassociate_julia_struct before
dropping the reference to the handle. But then we also fully address any
remaining race condition by having uvfinalize acquire a lock also.

The uv_return_spawn callback also needs to be synchronized with the
constructor, since we might have arrived there before we finished
allocating the Process struct here, leading to missed exit events.

Fixes #44460

base/process.jl

@@ -38,9 +38,13 @@ pipe_writer(p::ProcessChain) = p.in
 # release ownership of the libuv handle
 function uvfinalize(proc::Process)
     if proc.handle != C_NULL
-        disassociate_julia_struct(proc.handle)
-        ccall(:jl_close_uv, Cvoid, (Ptr{Cvoid},), proc.handle)
-        proc.handle = C_NULL
+        iolock_begin()
+        if proc.handle != C_NULL
+            disassociate_julia_struct(proc.handle)
+            ccall(:jl_close_uv, Cvoid, (Ptr{Cvoid},), proc.handle)
+            proc.handle = C_NULL
+        end
+        iolock_end()
     end
     nothing
 end
@@ -52,6 +56,7 @@ function uv_return_spawn(p::Ptr{Cvoid}, exit_status::Int64, termsignal::Int32)
     proc = unsafe_pointer_to_objref(data)::Process
     proc.exitcode = exit_status
     proc.termsignal = termsignal
+    disassociate_julia_struct(proc) # ensure that data field is set to C_NULL
     ccall(:jl_close_uv, Cvoid, (Ptr{Cvoid},), proc.handle)
     proc.handle = C_NULL
     lock(proc.exitnotify)
@@ -95,8 +100,9 @@ end
             end
             for io in stdio]
         handle = Libc.malloc(_sizeof_uv_process)
-        disassociate_julia_struct(handle) # ensure that data field is set to C_NULL
+        disassociate_julia_struct(handle)
         (; exec, flags, env, dir) = cmd
+        iolock_begin()
         err = ccall(:jl_spawn, Int32,
                   (Cstring, Ptr{Cstring}, Ptr{Cvoid}, Ptr{Cvoid},
                    Ptr{Tuple{Cint, UInt}}, Int,
@@ -109,13 +115,17 @@ end
             cpumask === nothing ? C_NULL : cpumask,
             cpumask === nothing ? 0 : length(cpumask),
             @cfunction(uv_return_spawn, Cvoid, (Ptr{Cvoid}, Int64, Int32)))
+        if err == 0
+            pp = Process(cmd, handle)
+            associate_julia_struct(handle, pp)
+        else
+            ccall(:jl_forceclose_uv, Cvoid, (Ptr{Cvoid},), handle) # will call free on handle eventually
+        end
+        iolock_end()
     end
     if err != 0
-        ccall(:jl_forceclose_uv, Cvoid, (Ptr{Cvoid},), handle) # will call free on handle eventually
         throw(_UVError("could not spawn " * repr(cmd), err))
     end
-    pp = Process(cmd, handle)
-    associate_julia_struct(handle, pp)
     return pp
 end
 

src/jl_uv.c

@@ -282,7 +282,7 @@ JL_DLLEXPORT void jl_uv_disassociate_julia_struct(uv_handle_t *handle)
     handle->data = NULL;
 }
 
-#define UV_CLOSED 0x02 // UV_HANDLE_CLOSED on Windows (same value)
+#define UV_HANDLE_CLOSED 0x02
 
 JL_DLLEXPORT int jl_spawn(char *name, char **argv,
                           uv_loop_t *loop, uv_process_t *proc,
@@ -308,7 +308,7 @@ JL_DLLEXPORT int jl_spawn(char *name, char **argv,
         if (!(flags == UV_INHERIT_FD || flags == UV_INHERIT_STREAM || flags == UV_IGNORE)) {
             proc->type = UV_PROCESS;
             proc->loop = loop;
-            proc->flags = UV_CLOSED;
+            proc->flags = UV_HANDLE_CLOSED;
             return UV_EINVAL;
         }
     }