Prevent exit callback instance leaks

De-references pty_baton's exit callback after it is called so it can be
automatically released, preventing leaked instances.

Adds basic implementation for Pipe's get_status() method and forces PTY
to wait for child process to exit to ensure exit callback is cleaned up.

Adds a test to check that exit callback is still called as usual.
This commit is contained in:
Leroy Hopson 2022-06-23 20:30:27 +07:00
parent 38927e0a3e
commit 0ae1d80abb
No known key found for this signature in database
GPG key ID: D2747312A6DB51AA
5 changed files with 24 additions and 4 deletions

View file

@ -31,6 +31,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
`pipe.close()` method of the gdnative library is now registered. `pipe.close()` method of the gdnative library is now registered.
- Fixed 'Condition "!obj" is true.' error that would often print to console when - Fixed 'Condition "!obj" is true.' error that would often print to console when
closing terminals in the Terminal panel of the editor plugin. closing terminals in the Terminal panel of the editor plugin.
- Fixed leaked instances that would occur when PTY exited but child process was still
running.
## [v2.0.0](https://github.com/lihop/godot-xterm/compare/v1.2.1...v2.0.0) - 2021-07-25 ## [v2.0.0](https://github.com/lihop/godot-xterm/compare/v1.2.1...v2.0.0) - 2021-07-25

View file

@ -445,8 +445,10 @@ static void pty_after_waitpid(uv_async_t *async) {
Array argv = Array::make(baton->exit_code, baton->signal_code); Array argv = Array::make(baton->exit_code, baton->signal_code);
if (baton->cb != nullptr && baton->cb->is_valid()) if (baton->cb != nullptr && baton->cb->is_valid()) {
baton->cb->call_funcv(argv); baton->cb->call_funcv(argv);
baton->cb = (Ref<FuncRef>)nullptr;
}
uv_close((uv_handle_t *)async, pty_after_close); uv_close((uv_handle_t *)async, pty_after_close);
} }

View file

@ -25,6 +25,7 @@ void Pipe::_register_methods() {
register_method("poll", &Pipe::_poll_connection); register_method("poll", &Pipe::_poll_connection);
register_method("open", &Pipe::open); register_method("open", &Pipe::open);
register_method("write", &Pipe::write); register_method("write", &Pipe::write);
register_method("get_status", &Pipe::get_status);
register_method("close", &Pipe::close); register_method("close", &Pipe::close);
register_signal<Pipe>("data_received", "data", register_signal<Pipe>("data_received", "data",
@ -56,11 +57,12 @@ godot_error Pipe::open(int fd, bool ipc = false) {
RETURN_IF_UV_ERR( RETURN_IF_UV_ERR(
uv_read_start((uv_stream_t *)&handle, _alloc_buffer, _read_cb)); uv_read_start((uv_stream_t *)&handle, _alloc_buffer, _read_cb));
status = 1;
return GODOT_OK; return GODOT_OK;
} }
void Pipe::close() { void Pipe::close() {
uv_close((uv_handle_t *)&handle, NULL); uv_close((uv_handle_t *)&handle, _close_cb);
uv_run(uv_default_loop(), UV_RUN_NOWAIT); uv_run(uv_default_loop(), UV_RUN_NOWAIT);
} }
@ -84,6 +86,8 @@ godot_error Pipe::write(String p_data) {
} }
int Pipe::get_status() { int Pipe::get_status() {
if (!uv_is_active((uv_handle_t *)&handle))
status = 0;
_poll_connection(); _poll_connection();
return status; return status;
} }
@ -120,3 +124,8 @@ void _alloc_buffer(uv_handle_t *handle, size_t suggested_size, uv_buf_t *buf) {
buf->base = (char *)malloc(suggested_size); buf->base = (char *)malloc(suggested_size);
buf->len = suggested_size; buf->len = suggested_size;
} }
void _close_cb(uv_handle_t *handle) {
Pipe *pipe = static_cast<Pipe *>(handle->data);
pipe->status = 0;
}

View file

@ -96,9 +96,10 @@ func open(cols: int = DEFAULT_COLS, rows: int = DEFAULT_ROWS) -> Array:
func _exit_tree(): func _exit_tree():
_exit_cb = null
if _pid > 1: if _pid > 1:
LibuvUtils.kill(_pid, Signal.SIGHUP) LibuvUtils.kill(_pid, Signal.SIGHUP)
while _pipe.get_status() != 0:
continue
func _on_pipe_data_received(data): func _on_pipe_data_received(data):

View file

@ -73,6 +73,12 @@ func test_closes_pty_on_exit():
assert_eq(new_num_pts, num_pts) assert_eq(new_num_pts, num_pts)
func test_emits_exited_signal_when_child_process_exits():
pty.call_deferred("fork", "exit")
yield(yield_to(pty, "exited", 1), YIELD)
assert_signal_emitted(pty, "exited")
class Helper: class Helper:
static func _get_pts() -> Array: static func _get_pts() -> Array:
assert(false, "Abstract method") assert(false, "Abstract method")