Skip to content

Commit

Permalink
avoid leaving process zombies
Browse files Browse the repository at this point in the history
while investigating JuliaLang#14434, I noticed that the test code there is creating zombies
to fix that, we need to avoid calling uv_close in the finalizer
until after the process has exited (or during atexit)
  • Loading branch information
vtjnash committed Aug 1, 2016
1 parent c4b1ed6 commit 67efdcf
Show file tree
Hide file tree
Showing 9 changed files with 37 additions and 14 deletions.
11 changes: 7 additions & 4 deletions base/process.jl
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ type Process <: AbstractPipe
typemin(fieldtype(Process, :termsignal)),
false, Condition(), false, Condition())
finalizer(this, uvfinalize)
this
return this
end
end
pipe_reader(p::Process) = p.out
Expand Down Expand Up @@ -325,9 +325,12 @@ function _jl_spawn(cmd, argv, loop::Ptr{Void}, pp::Process,
end

function uvfinalize(proc::Process)
proc.handle != C_NULL && ccall(:jl_close_uv, Void, (Ptr{Void},), proc.handle)
disassociate_julia_struct(proc)
proc.handle = C_NULL
if proc.handle != C_NULL
disassociate_julia_struct(proc.handle)
ccall(:jl_close_uv, Void, (Ptr{Void},), proc.handle)
proc.handle = C_NULL
end
nothing
end

function uv_return_spawn(p::Ptr{Void}, exit_status::Int64, termsignal::Int32)
Expand Down
4 changes: 2 additions & 2 deletions base/socket.jl
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ function TCPSocket()
throw(UVError("failed to create tcp socket",err))
end
this.status = StatusInit
this
return this
end

type TCPServer <: LibuvServer
Expand Down Expand Up @@ -312,7 +312,7 @@ function TCPServer()
throw(UVError("failed to create tcp server",err))
end
this.status = StatusInit
this
return this
end

isreadable(io::TCPSocket) = isopen(io) || nb_available(io) > 0
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
c6a019d79d20eabc39619a04961c9a3b
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
478ab473244b01bef344892a75e09fef50da8fb1a7212e0257c53f3223de4fde5f6bd449eef34bc1f025481c7d9f854002acb6eb203b447a50a34bae4ad9dee4

This file was deleted.

This file was deleted.

2 changes: 1 addition & 1 deletion deps/libuv.version
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
LIBUV_BRANCH=julia-uv1.9.0
LIBUV_SHA1=ecbd6eddfac4940ab8db57c73166a7378563ebd3
LIBUV_SHA1=cb6d0f875a5b8ca30cba45c0c1ef7442c87c1e68
9 changes: 8 additions & 1 deletion src/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ static struct uv_shutdown_queue_item *next_shutdown_queue_item(struct uv_shutdow

void jl_init_timing(void);
void jl_destroy_timing(void);
void jl_uv_call_close_callback(jl_value_t *val);

JL_DLLEXPORT void jl_atexit_hook(int exitcode)
{
Expand Down Expand Up @@ -270,6 +271,13 @@ JL_DLLEXPORT void jl_atexit_hook(int exitcode)
continue;
}
switch(handle->type) {
case UV_PROCESS:
// cause Julia to forget about the Process object
if (handle->data)
jl_uv_call_close_callback((jl_value_t*)handle->data);
// and make libuv think it is already dead
((uv_process_t*)handle)->pid = 0;
// fall-through
case UV_TTY:
case UV_UDP:
case UV_TCP:
Expand All @@ -283,7 +291,6 @@ JL_DLLEXPORT void jl_atexit_hook(int exitcode)
case UV_PREPARE:
case UV_CHECK:
case UV_SIGNAL:
case UV_PROCESS:
case UV_FILE:
// These will be shutdown as appropriate by jl_close_uv
jl_close_uv(handle);
Expand Down
21 changes: 17 additions & 4 deletions src/jl_uv.c
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,17 @@ void jl_init_signal_async(void)
}
#endif

static void jl_uv_call_close_callback(jl_value_t *val)
void jl_uv_call_close_callback(jl_value_t *val)
{
jl_value_t *args[2];
args[0] = jl_get_global(jl_base_relative_to(((jl_datatype_t*)jl_typeof(val))->name->module),
jl_symbol("_uv_hook_close")); // topmod(typeof(val))._uv_hook_close
args[1] = val;
assert(args[0]);
jl_apply(args, 2);
jl_apply(args, 2); // TODO: wrap in try-catch?
}

JL_DLLEXPORT void jl_uv_closeHandle(uv_handle_t *handle)
static void jl_uv_closeHandle(uv_handle_t *handle)
{
// if the user killed a stdio handle,
// revert back to direct stdio FILE* writes
Expand All @@ -107,7 +107,7 @@ JL_DLLEXPORT void jl_uv_closeHandle(uv_handle_t *handle)
free(handle);
}

JL_DLLEXPORT void jl_uv_shutdownCallback(uv_shutdown_t *req, int status)
static void jl_uv_shutdownCallback(uv_shutdown_t *req, int status)
{
/*
* This happens if the remote machine closes the connecition while we're
Expand Down Expand Up @@ -180,8 +180,21 @@ JL_DLLEXPORT int jl_init_pipe(uv_pipe_t *pipe, int writable, int readable,
return err;
}

static void jl_proc_exit_cleanup(uv_process_t *process, int64_t exit_status, int term_signal)
{
uv_close((uv_handle_t*)process, (uv_close_cb)&free);
}

JL_DLLEXPORT void jl_close_uv(uv_handle_t *handle)
{
if (handle->type == UV_PROCESS && ((uv_process_t*)handle)->pid != 0) {
// take ownership of this handle,
// so we can waitpid for the resource to exit and avoid leaving zombies
assert(handle->data == NULL); // make sure Julia has forgotten about it already
((uv_process_t*)handle)->exit_cb = jl_proc_exit_cleanup;
return;
}

if (handle->type == UV_FILE) {
uv_fs_t req;
jl_uv_file_t *fd = (jl_uv_file_t*)handle;
Expand Down

0 comments on commit 67efdcf

Please sign in to comment.