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

Wezterm serial uses 100% CPU #1594

Open
juho-p opened this issue Jan 27, 2022 · 4 comments
Open

Wezterm serial uses 100% CPU #1594

juho-p opened this issue Jan 27, 2022 · 4 comments
Labels
bug Something isn't working

Comments

@juho-p
Copy link

juho-p commented Jan 27, 2022

What Operating System(s) are you seeing this problem on?

Linux X11

WezTerm version

20220126-105230-0c6b4e26

Did you try the latest nightly build to see if the issue is better (or worse!) than your current version?

Yes, and I updated the version box above to show the version of the nightly that I tried

Describe the bug

Using wezterm serial terminal functionality uses all the CPU of single thread. Tried bot appimage version and also build from source (both had the same issue)

WEZTERM_LOG=trace wezterm serial --baud 115200 /dev/ttyUSB0 2> wezterm-serial-trace-log.txt

htop shows wezterm-gui process using 100% cpu (and laptop fans yell like crazy)

Also, before serial session starts, "[Process didn't exit cleanly. (exit_behavior="CloseOnCleanExit")]" is printed to console. Have to press enter before serial terminal is functional.

I have this only when using the serial. Otherwise the serial terminal seems to work just fine.

For comparison, screen /dev/ttyUSB0 115200 does not have the CPU usage issue (but it's not fun to use).

Attached a log from above command. There is not "infinite log printing" there, but maybe something else can be figured out:

wezterm-serial-trace-log.txt

To Reproduce

  • Start wezterm serial session
  • observe CPU usage

Configuration

no config

Expected Behavior

wezterm serial should not use 100% CPU

Logs

wezterm-serial-trace-log.txt

Anything else?

No response

@juho-p juho-p added the bug Something isn't working label Jan 27, 2022
@juho-p
Copy link
Author

juho-p commented Feb 3, 2022

I did some brief debugging on this and the busy thread seems to run infinite loop without delay in src/pty/serial.rs:225

Adding some debug println! there fills the terminal with that print output. Seems self.fd.read returns immediately with 0 bytes read.

@wez
Copy link
Owner

wez commented Feb 3, 2022

I haven't had a chance to sit down with a serial device to look at this yet, but that looping is expected because each
read has a timeout configured:

// The timeout needs to be rather short because, at least on Windows,
// a read with a long timeout will block a concurrent write from
// happening. In wezterm we tend to have a thread looping on read
// while writes happen occasionally from the gui thread, and if we
// make this timeout too long we can block the gui thread.
port.set_timeout(Duration::from_millis(50))?;

You may want to try say doubling that timeout and see if it helps. I have a suspicion that there might be something
else contributing to the CPU utilization.

@juho-p
Copy link
Author

juho-p commented Feb 4, 2022

I'm quite sure that loop is the reason and that specific thrad takes all the CPU it can.

I can verify this theory by adding a let _ = std::thread::sleep(Duration::from_millis(50)); at the start of the loop. Doing so takes CPU usage down to ~0. (doing so also makes the terminal output laggy, so it's terrible "fix").

Changing that timeout has no effect at all on the behaviour of that loop. I think this is because the time out is not a property of file descriptor, but a feature of the serial-core library.

I did some testing, and using the terminal through TTYPort implementation makes that timeout actually work. You can see the (not portable) change here: juho-p@9da8137

My attempt to fix it in that commit is not actually nice at all, since that makes the terminal input very laggy, which I think is because now the fd mutex is locked most of the time by that loop (only unlocks every 50ms). It does take the CPU usage down again, hovever.

Right now I'm thinking maybe there could be just a poll to that file descriptor read, but I haven't tested if that works and I also think it wouldn't work on Windows?

wez added a commit that referenced this issue Feb 12, 2022
The serial port is always in non-blocking mode so we need to use our
own timeout and `poll(2)`.

While we're in here, the `wait` method could cause the gui to exit
immediately on startup because we'd interpret its immediate error
return result as child process completion.

This commit makes the wait method look at the carrier detect signal
and propagates IO errors back as completion events.

refs: #1594
@wez
Copy link
Owner

wez commented Feb 12, 2022

I've fixed this on unix systems. I've been poking at Windows for a bit and I can't seem to convince it to not to block indefinitely on writes. I think I might have to remove the serial crate dep on Windows and just use overlapped IO there.

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