diff --git a/addons/godot_xterm/native/src/pipe.cpp b/addons/godot_xterm/native/src/pipe.cpp index d313c1d..403f1db 100644 --- a/addons/godot_xterm/native/src/pipe.cpp +++ b/addons/godot_xterm/native/src/pipe.cpp @@ -66,20 +66,18 @@ void Pipe::close() { uv_run(uv_default_loop(), UV_RUN_NOWAIT); } -godot_error Pipe::write(String p_data) { - char *s = p_data.alloc_c_string(); - ULONG len = strlen(s); +godot_error Pipe::write(PoolByteArray data) { + char *s = (char *)data.read().ptr(); + ULONG len = data.size(); - uv_buf_t bufs[1]; - bufs[0].base = s; - bufs[0].len = len; + uv_buf_t buf; + uv_write_t *req = (uv_write_t *)malloc(sizeof(uv_write_t)); - uv_write_t req; - - req.data = s; - - uv_write(&req, (uv_stream_t *)&handle, bufs, 1, _write_cb); + buf.base = s; + buf.len = len; + req->data = (void *)buf.base; + uv_write(req, (uv_stream_t *)&handle, &buf, 1, _write_cb); uv_run(uv_default_loop(), UV_RUN_NOWAIT); return GODOT_OK; @@ -92,10 +90,16 @@ int Pipe::get_status() { return status; } -void Pipe::_poll_connection() { uv_run(uv_default_loop(), UV_RUN_NOWAIT); } +void Pipe::_poll_connection() { + if (status == 1 && !uv_is_active((uv_handle_t *)&handle)) + uv_read_start((uv_stream_t *)&handle, _alloc_buffer, _read_cb); + + uv_run(uv_default_loop(), UV_RUN_NOWAIT); +} void _read_cb(uv_stream_t *handle, ssize_t nread, const uv_buf_t *buf) { Pipe *pipe = static_cast(handle->data); + if (nread < 0) { switch (nread) { case UV_EOF: @@ -104,6 +108,7 @@ void _read_cb(uv_stream_t *handle, ssize_t nread, const uv_buf_t *buf) { // Can happen when the process exits. // As long as PTY has caught it, we should be fine. uv_read_stop(handle); + pipe->status = 0; return; default: UV_ERR_PRINT(nread); @@ -113,12 +118,19 @@ void _read_cb(uv_stream_t *handle, ssize_t nread, const uv_buf_t *buf) { PoolByteArray data; data.resize(nread); - memcpy(data.write().ptr(), buf->base, nread); + { memcpy(data.write().ptr(), buf->base, nread); } + std::free((char *)buf->base); pipe->emit_signal("data_received", data); + + // Stop reading until the next poll, otherwise _read_cb could be called + // repeatedly, blocking Godot, and eventually resulting in a memory pool + // allocation error. This can be triggered with the command `cat /dev/urandom` + // if reading is not stopped. + uv_read_stop(handle); } -void _write_cb(uv_write_t *req, int status) {} +void _write_cb(uv_write_t *req, int status) { std::free(req); } void _alloc_buffer(uv_handle_t *handle, size_t suggested_size, uv_buf_t *buf) { buf->base = (char *)malloc(suggested_size); @@ -128,4 +140,4 @@ void _alloc_buffer(uv_handle_t *handle, size_t suggested_size, uv_buf_t *buf) { void _close_cb(uv_handle_t *handle) { Pipe *pipe = static_cast(handle->data); pipe->status = 0; -} \ No newline at end of file +} diff --git a/addons/godot_xterm/native/src/pipe.h b/addons/godot_xterm/native/src/pipe.h index 0b7c09a..19ebb5a 100644 --- a/addons/godot_xterm/native/src/pipe.h +++ b/addons/godot_xterm/native/src/pipe.h @@ -26,7 +26,7 @@ public: void close(); int get_status(); - godot_error write(String p_data); + godot_error write(PoolByteArray data); void pause(); void resume(); diff --git a/addons/godot_xterm/native/src/terminal.cpp b/addons/godot_xterm/native/src/terminal.cpp index 0d67f81..7084e8d 100644 --- a/addons/godot_xterm/native/src/terminal.cpp +++ b/addons/godot_xterm/native/src/terminal.cpp @@ -224,19 +224,17 @@ static void term_output(const char *s, size_t len, void *user) {} static void write_cb(struct tsm_vte *vte, const char *u8, size_t len, void *data) { - Terminal *term = static_cast(data); PoolByteArray bytes = PoolByteArray(); - - for (int i = 0; i < len; i++) - bytes.append(u8[i]); + bytes.resize(len); + { memcpy(bytes.write().ptr(), u8, len); } if (len > 0) { if (term->input_event_key.is_valid()) { // The callback was fired from a key press event so emit the "key_pressed" // signal. - term->emit_signal("key_pressed", String(u8), term->input_event_key); + term->emit_signal("key_pressed", bytes, term->input_event_key); term->input_event_key.unref(); } @@ -263,7 +261,7 @@ static int text_draw_cb(struct tsm_screen *con, uint64_t id, const uint32_t *ch, if (len < 1) // No foreground to draw. return 0; - size_t ulen; + size_t ulen = 0; char buf[5] = {0}; char *utf8 = tsm_ucs4_to_utf8_alloc(ch, len, &ulen); @@ -310,8 +308,9 @@ void Terminal::_register_methods() { register_signal("data_sent", "data", GODOT_VARIANT_TYPE_POOL_BYTE_ARRAY); - register_signal("key_pressed", "data", GODOT_VARIANT_TYPE_STRING, - "event", GODOT_VARIANT_TYPE_OBJECT); + register_signal("key_pressed", "data", + GODOT_VARIANT_TYPE_POOL_BYTE_ARRAY, "event", + GODOT_VARIANT_TYPE_OBJECT); register_signal("size_changed", "new_size", GODOT_VARIANT_TYPE_VECTOR2); register_signal("bell", Dictionary()); @@ -609,11 +608,8 @@ void Terminal::update_size() { emit_signal("size_changed", Vector2(cols, rows)); } -void Terminal::write(String data) { - const char *u8 = data.alloc_c_string(); - size_t len = strlen(u8); - - tsm_vte_input(vte, u8, len); +void Terminal::write(PoolByteArray data) { + tsm_vte_input(vte, (char *)data.read().ptr(), data.size()); } void Terminal::sb_up(int num) { diff --git a/addons/godot_xterm/native/src/terminal.h b/addons/godot_xterm/native/src/terminal.h index 7757bd3..47c364a 100644 --- a/addons/godot_xterm/native/src/terminal.h +++ b/addons/godot_xterm/native/src/terminal.h @@ -53,7 +53,7 @@ public: void _gui_input(Variant event); void _draw(); - void write(String data); + void write(PoolByteArray data); void sb_up(int num); void sb_down(int num); diff --git a/addons/godot_xterm/nodes/pty/unix/pty_unix.gd b/addons/godot_xterm/nodes/pty/unix/pty_unix.gd index 3f12859..789c143 100644 --- a/addons/godot_xterm/nodes/pty/unix/pty_unix.gd +++ b/addons/godot_xterm/nodes/pty/unix/pty_unix.gd @@ -68,13 +68,12 @@ var _exit_cb: FuncRef # Writes data to the socket. # data: The data to write. func write(data) -> void: - assert(data is String or data is PoolByteArray) - - if data is PoolByteArray: - data = data.get_string_from_utf8() - + assert( + data is PoolByteArray or data is String, + "Invalid type for argument 'data'. Should be of type PoolByteArray or String" + ) if _pipe: - _pipe.write(data) + _pipe.write(data if data is PoolByteArray else data.to_utf8()) func resize(cols: int, rows: int) -> void: diff --git a/addons/godot_xterm/terminal.gd b/addons/godot_xterm/terminal.gd index 6f7c14f..3b789d4 100644 --- a/addons/godot_xterm/terminal.gd +++ b/addons/godot_xterm/terminal.gd @@ -78,7 +78,10 @@ func get_cols() -> int: func write(data) -> void: - assert(data is String or data is PoolByteArray) + assert( + data is PoolByteArray or data is String, + "Invalid type for argument 'data'. Should be of type PoolByteArray or String." + ) # Will be cleared when _flush() is called after VisualServer emits the "frame_pre_draw" signal. _buffer.push_back(data) @@ -90,7 +93,7 @@ func write(data) -> void: func _flush(): for data in _buffer: - _native_terminal.write(data if data is String else data.get_string_from_utf8()) + _native_terminal.write(data if data is PoolByteArray else data.to_utf8()) _native_terminal.update() _buffer.clear() @@ -269,8 +272,8 @@ func _on_data_sent(data: PoolByteArray): emit_signal("data_sent", data) -func _on_key_pressed(data: String, event: InputEventKey): - emit_signal("key_pressed", data, event) +func _on_key_pressed(data: PoolByteArray, event: InputEventKey): + emit_signal("key_pressed", data.get_string_from_utf8(), event) func _on_size_changed(new_size: Vector2): diff --git a/test/integration/terminal.test.gd b/test/integration/terminal.test.gd new file mode 100644 index 0000000..dabcbe6 --- /dev/null +++ b/test/integration/terminal.test.gd @@ -0,0 +1,9 @@ +extends "res://addons/gut/test.gd" + +const Terminal := preload("res://addons/godot_xterm/terminal.gd") + + +func test_writing_random_data_to_terminal_does_not_crash_application(): + add_child_autofree(preload("res://test/scenes/write_random.tscn").instance()) + yield(yield_frames(5, "Writing random data to terminal"), YIELD) + assert_true(true, "Expected no crash when writing random data to terminal.") diff --git a/test/scenes/write_random.tscn b/test/scenes/write_random.tscn new file mode 100644 index 0000000..32827f2 --- /dev/null +++ b/test/scenes/write_random.tscn @@ -0,0 +1,21 @@ +[gd_scene load_steps=2 format=2] + +[sub_resource type="GDScript" id=1] +script/source = "extends \"res://addons/godot_xterm/terminal.gd\" + +var rng := RandomNumberGenerator.new() + + +func _ready(): + rng.seed = 0 + + +func _process(_delta): + for _i in range(4096): + write(PoolByteArray([rng.randi() % 256])) +" + +[node name="Terminal" type="Control"] +anchor_right = 1.0 +anchor_bottom = 1.0 +script = SubResource( 1 )