Skip to content

Commit 19ce653

Browse files
vtjnashKristofferC
authored andcommitted
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 (cherry picked from commit c591bf2)
1 parent 225cb1d commit 19ce653

File tree

2 files changed

+19
-9
lines changed

2 files changed

+19
-9
lines changed

base/process.jl

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,13 @@ pipe_writer(p::ProcessChain) = p.in
3838
# release ownership of the libuv handle
3939
function uvfinalize(proc::Process)
4040
if proc.handle != C_NULL
41-
disassociate_julia_struct(proc.handle)
42-
ccall(:jl_close_uv, Cvoid, (Ptr{Cvoid},), proc.handle)
43-
proc.handle = C_NULL
41+
iolock_begin()
42+
if proc.handle != C_NULL
43+
disassociate_julia_struct(proc.handle)
44+
ccall(:jl_close_uv, Cvoid, (Ptr{Cvoid},), proc.handle)
45+
proc.handle = C_NULL
46+
end
47+
iolock_end()
4448
end
4549
nothing
4650
end
@@ -52,6 +56,7 @@ function uv_return_spawn(p::Ptr{Cvoid}, exit_status::Int64, termsignal::Int32)
5256
proc = unsafe_pointer_to_objref(data)::Process
5357
proc.exitcode = exit_status
5458
proc.termsignal = termsignal
59+
disassociate_julia_struct(proc) # ensure that data field is set to C_NULL
5560
ccall(:jl_close_uv, Cvoid, (Ptr{Cvoid},), proc.handle)
5661
proc.handle = C_NULL
5762
lock(proc.exitnotify)
@@ -95,8 +100,9 @@ end
95100
end
96101
for io in stdio]
97102
handle = Libc.malloc(_sizeof_uv_process)
98-
disassociate_julia_struct(handle) # ensure that data field is set to C_NULL
103+
disassociate_julia_struct(handle)
99104
(; exec, flags, env, dir) = cmd
105+
iolock_begin()
100106
err = ccall(:jl_spawn, Int32,
101107
(Cstring, Ptr{Cstring}, Ptr{Cvoid}, Ptr{Cvoid},
102108
Ptr{Tuple{Cint, UInt}}, Int,
@@ -109,13 +115,17 @@ end
109115
cpumask === nothing ? C_NULL : cpumask,
110116
cpumask === nothing ? 0 : length(cpumask),
111117
@cfunction(uv_return_spawn, Cvoid, (Ptr{Cvoid}, Int64, Int32)))
118+
if err == 0
119+
pp = Process(cmd, handle)
120+
associate_julia_struct(handle, pp)
121+
else
122+
ccall(:jl_forceclose_uv, Cvoid, (Ptr{Cvoid},), handle) # will call free on handle eventually
123+
end
124+
iolock_end()
112125
end
113126
if err != 0
114-
ccall(:jl_forceclose_uv, Cvoid, (Ptr{Cvoid},), handle) # will call free on handle eventually
115127
throw(_UVError("could not spawn " * repr(cmd), err))
116128
end
117-
pp = Process(cmd, handle)
118-
associate_julia_struct(handle, pp)
119129
return pp
120130
end
121131

src/jl_uv.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ JL_DLLEXPORT void jl_uv_disassociate_julia_struct(uv_handle_t *handle)
282282
handle->data = NULL;
283283
}
284284

285-
#define UV_CLOSED 0x02 // UV_HANDLE_CLOSED on Windows (same value)
285+
#define UV_HANDLE_CLOSED 0x02
286286

287287
JL_DLLEXPORT int jl_spawn(char *name, char **argv,
288288
uv_loop_t *loop, uv_process_t *proc,
@@ -308,7 +308,7 @@ JL_DLLEXPORT int jl_spawn(char *name, char **argv,
308308
if (!(flags == UV_INHERIT_FD || flags == UV_INHERIT_STREAM || flags == UV_IGNORE)) {
309309
proc->type = UV_PROCESS;
310310
proc->loop = loop;
311-
proc->flags = UV_CLOSED;
311+
proc->flags = UV_HANDLE_CLOSED;
312312
return UV_EINVAL;
313313
}
314314
}

0 commit comments

Comments
 (0)