Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Unicode error: no space left" cause strange behaviors #55

Closed
ConteZero opened this issue Aug 9, 2022 · 4 comments
Closed

"Unicode error: no space left" cause strange behaviors #55

ConteZero opened this issue Aug 9, 2022 · 4 comments
Labels
bug Something isn't working

Comments

@ConteZero
Copy link

While executing some commands the following errors appear in the logs:

Unicode error: no space left
Unicode error: invalid skip

Some texts are not displayed in the terminal and sometimes strange behaviors appear on the screen.
test-2022-08-09.zip

I've uploaded a zipped mp4 file because github do not recognize it as valid mp4 file.
I've used Godot 3.5 and man scout is not the only command that has this problem, in my system it happens with other commands too, here are some examples:

man -Len -Tutf8 virt-edit
man -Len -Tutf8 dialog
man -Len -Tutf8 systemd-xdg-autostart-generator
man -Len -Tutf8 firewall-cmd
@lihop lihop added the bug Something isn't working label Aug 9, 2022
@lihop
Copy link
Owner

lihop commented Aug 10, 2022

@ConteZero I can confirm the "Unicode error" messages and that they occur when texts are not displayed:

issue3

I'm still digging into the cause of these errors and trying to find a way to reproduce them reliably. Some of the other strange behavior in the video you posted (shown below) can be explained by a new issue I've created #56 (Incorrect intial PTY size).

2022-08-11-100644_802x460_scrot

@ConteZero
Copy link
Author

I can replicate the error with this file:
log_man_virt-edit.txt

It is the result of this command man -Len -Tutf8 --nh --nj virt-edit > log_man_virt-edit.txt and if I execute cat log_man_virt-edit.txt I get the error on the log and the output is truncated.

I think the problem is the • character that appear the first time on line 294.
If I add more text after line 294 the problem keeps popping up, but if I add 2 or more characters before that line the problem disappears (with only 1 character the problem remains).

lihop added a commit that referenced this issue Aug 14, 2022
Changes `write()` method of native pipe and terminal to accept a
PoolByteArray rather than String. This means that `get_string_from_utf8()`
is no longer called on data coming from PTY and being sent to Terminal.

The terminal state machine already has a UTF8 parser which maintains
its state across calls to `write()`. This means that we can write half
the bytes of a single unicode character in one call and the remaining half
in the next call and the state machine will parse it correctly.

On the other hand, the `get_string_from_utf8()` method of Godot's
PoolByteArray requires that the array contains completely valid UTF8,
otherwise we get errors such as "Unicode error: invalid skip".

The data coming from PTY can be arbitrarily split in the middle of a
unicode character meaning that we will sometimes get errors when calling
`get_string_from_utf8()` on it. This is more likely to occur when there
is a large amount of output (i.e. it's more likely to be split). In other
cases, the data might intentionally contain invalid unicode such as when
printing binary files or random data (e.g. `cat /bin/sh`, `cat /dev/random`).

We avoid these errors by passing the PoolByteArray data directly to the terminal
state machine.

In addition to fixing unicode errors, this commit:
- Prevents repeated calls to pipes `_read_cb()` method that would block Godot
  and result in a crash with the message "ERROR: All memory pool allocations
  are in use" that resulted from writing data to an ever-increasing number of
  PoolByteArrays before any of them could be freed. This could be triggered by
  running the `cat /dev/urandom` command after making the change to `write()`
  mentioned above.
- Prevents memory leaks by freeing libuv buffers after they have been copied
  to PoolByteArrays.

Fixes #55.
@lihop
Copy link
Owner

lihop commented Aug 14, 2022

@ConteZero I think I've fixed this issue in 3ad0e10 and issue #56 in f774c90. I have released a beta with these patches along with patches for issues #51 and #53.

Could you install version 2.1.1-beta1 and let me know if all the issues you have reported are fixed? I don't know how you installed GodotXterm, but the GDNative code has changed, so you will need to recompile the binaries or download the new ones from the releases page.

If everything is working as expected and you have no more issues to report, then I plan to go ahead and release bugfix-version 2.1.1.

Thanks for your feedback so far, it is much appreciated.

@ConteZero
Copy link
Author

I did some tests and I can confirm that all issues are solved for me.

With version 2.1.1-beta1 I get the following error on the log console when the project is loaded but it does not appear to cause problems when you run it:
res://addons/godot_xterm/pty.gd:105 - Invalid get index 'cols' (on base: 'Control (terminal.gd)').

@lihop lihop closed this as completed in 9ed6750 Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants