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

Very high CPU usage on fish 3.2 when displaying large strings #7837

Closed
henrikhorluck opened this issue Mar 19, 2021 · 17 comments
Closed

Very high CPU usage on fish 3.2 when displaying large strings #7837

henrikhorluck opened this issue Mar 19, 2021 · 17 comments
Labels
bug Something that's not working as intended
Milestone

Comments

@henrikhorluck
Copy link
Contributor

henrikhorluck commented Mar 19, 2021

Hi! I am having an issue with strings in commands that are longer than ~1048 characters is causing 100% CPU usage on 1 or more cores. I have confirmed that it is not an issue with fish 3.1.2, and it reproduces when using sh -c 'env HOME=$(mktemp -d) fish'

Version-stuff (was also a problem on 3.2.0):

> fish --version
fish, version 3.2.1
> echo $version
v3.2.1
> uname -u
Darwin Henriks-Macbook.local 20.3.0 Darwin Kernel Version 20.3.0: Thu Jan 21 00:07:06 PST 2021; root:xnu-7195.81.3~1/RELEASE_X86_64 x86_64

Example of command that triggers high CPU-usage just by being pasted into the terminal, or by being written yourself, it triggers simply being displayed.

touch "qg5e6rnmfzQZh18hMUbgDiwHfN+Abd22mIWuuMYemSvfdetIog4B5HPJgTC5hAkwNMPpn+TuRqJnvn3EVWkZN42ksyH/WawZ6Zq5Z+EwAJLs5dzVuzw6cwI2UfHWIZrew90W4XG8Ad/y+rwPt4iHVRIYaSkIGj6mzsD0w7rJdvHpIbeNDUcggCWlR80wsd4IrhoLFdw7Okhy0vj36tOd9M18uQpFJENEd1zFWUaKiV9PFBZepS71rqEeuUZ8K5JgGBqlOoX3qePy9ryd+AMGAsBKi/ivxekj05qFSOuHgGWBabNlyQJfGhQt81QD3s8DRS+QH2fUaiTrLDfWhwu1qg5e6rnmfzQZh18hMUbgDiwHfN+Abd22mIWuuMYemSvfdetIog4B5HPJgTC5hAkwNMPpn+TuRqJnvn3EVWkZN42ksyH/WawZ6Zq5Z+EwAJLs5dzVuzw6cwI2UfHWIZrew90W4XG8Ad/y+rwPt4iHVRIYaSkIGj6mzsD0w7rJdvHpIbeNDUcggCWlR80wsd4IrhoLFdw7Okhy0vj36tOd9M18uQpFJENEd1zFWUaKiV9PFBZepS71rqEeuUZ8K5JgGBqlOoX3qePy9ryd+AMGAsBKi/ivxekj05qFSOuHgGWBabNlyQJfGhQt81QD3s8DRS+QH2fUaiTrLDfWhwu1qg5e6rnmfzQZh18hMUbgDiwHfN+Abd22mIWuuMYemSvfdetIog4B5HPJgTC5hAkwNMPpn+TuRqJnvn3EVWkZN42ksyH/WawZ6Zq5Z+EwAJLs5dzVuzw6cwI2UfHWIZrew90W4XG8Ad/y+rwPt4iHVRIYaSkIGj6mzsD0w7rJdvHpIbeNDUcggCWlR80wsd4IrhoLFdw7Okhy0vj36tOd9M18uQpFJENEd1zFWUaKiV9PFBZepS71rqEeuUZ8K5JgGBqlOoX3qePy9ryd+AMGAsBKi/ivxekj05qFSOuHgGWBabNlyQJfGhQt81QD3s8DRS+QH2fUaiTrLDfWhwu132TuRqJnvn3EVWkZN42ksyH/w

If you have one less character in the string it does not cause the high CPU-usage. One thing I noticed when testing fish 3.1.2 was that it didn't add line breaks to the commands, while 3.2.1 had it over multiple lines.

Please let me know if something is unclear, you need any more information to reproduce it, or debug information I can supply

@faho
Copy link
Member

faho commented Mar 19, 2021

I can't reproduce on linux.

Which terminal are you using? Does this also happen in other terminals? Any customization (including prompts, vi-mode, third-party packages)?

Does the text matter at all? Does this also happen when you just have, say, 2000 "a"s (string repeat -n 2000 a)? Unquoted, inside single-quotes?

@henrikhorluck
Copy link
Contributor Author

henrikhorluck commented Mar 19, 2021

I am using the built in terminal on macOS, VI-mode and tmux (v.3.1c).

It must be after a command, so e.g. cd or touch first, while "<characters>" on itself doesn't trigger it. The text inside the string does not matter, I also do not think it needs quotes, so touch <lots of a> also triggers it.

Perhaps relevant stuff from config.fish

if status --is-interactive
	tmux ^ /dev/null; and exec true
end

# Base16 Shell
# from https://github.com/chriskempson/base16-shell
if status --is-interactive
    set BASE16_SHELL "$HOME/.config/base16-shell/"
    source "$BASE16_SHELL/profile_helper.fish"
end

# Fish should not add things to clipboard when killing
# See https://github.com/fish-shell/fish-shell/issues/772
set FISH_CLIPBOARD_CMD "cat"

Update: It also reproduces in Alacritty

@faho
Copy link
Member

faho commented Mar 19, 2021

It must be after a command

What do you mean by that?

Have you tried disabling vi-mode and commenting out your config.fish?

Sidenote: FISH_CLIPBOARD_CMD has been dead for 4.5 years by this point, and the ^ redirection is deprecated (use 2> instead).

@faho
Copy link
Member

faho commented Mar 19, 2021

Ah, I think you're not getting bracketed paste for some reason - this tells fish that the text it's getting is pasted, so it can skip a bunch of things (like, say, executing it when there's a newline, but also highlighting).

Since you're using tmux, and from fish's point of view that's its terminal, you should check if that is disabling it for some reason (fish enables it unconditionally).

@faho
Copy link
Member

faho commented Mar 19, 2021

If you use tmux's pasting, you need to use paste-buffer -p (because of course there is no way for tmux to detect that... oh, wait, it detects if it's off and then doesn't do it even with "-p". lovely)

@ross2084
Copy link

I have the same problem without tmux, in fish version 3.2.1. I'm not sure, but I think the problem appeared in 3.2.0. Pasting the original touch command text causes my shell to immediately begin consuming at least 100% CPU.

Darwin 20G-T0WXJG5M-35K 19.6.0 Darwin Kernel Version 19.6.0: Tue Nov 10 00:10:30 PST 2020; root:xnu-6153.141.10~1/RELEASE_X86_64 x86_64

@faho
Copy link
Member

faho commented Mar 19, 2021

Alright, this seems like mac-specific stuff, so I can't be of any more assistance.

@henrikhorluck
Copy link
Contributor Author

Okey, it reproduces without tmux, and with commented out config.fish, and fish_user_default_keybindings.

By it must be after a command I mean that

> aaaaaaaaaaa (a little longer)

Doesn't trigger the issue at hand, but

> touch aaaaaaaaaaaa

Does trigger it

@jsatk
Copy link

jsatk commented Mar 19, 2021

This is also happening for me as well.
Screenshot of Activity Monitor on macOS showing Fish shell using 1200% CPU

@ross2084
Copy link

ross2084 commented Mar 19, 2021

One additional bit of possibly useful behavior I've noticed is that even when typing at the shell prompt, % CPU spikes quite a bit until I stop typing, where it then drops back down again. The faster I type, the higher the percentage increase.

With Zsh or Bash, % CPU jumps only slightly if I rapidly type at my prompt, maybe a tenth or a fifth of a percent increase. But Fish jumps well into the 20'ish percent from simply typing text at rapid speed.

@krobelus
Copy link
Member

Can you reproduce inside HOME=(mktemp -d) fish?

when typing at the shell prompt, % CPU spikes quite a bit until I stop typing

Brief spikes are expected and harmless; fish is just computing autosuggestions (from shell history).

@ross2084
Copy link

Thanks for the clarification on the temporary spikes.

Yes I can reproduce with HOME=(mktemp -d) fish. Pasting the above example text causes an immediate spike to 100% or more CPU until the shell is closed.

@zanchey
Copy link
Member

zanchey commented Mar 20, 2021

I can reproduce this on macOS with the string above.

The sample shows spending two threads spending their whole time (2681 samples) in each of the following:

Call graph:
    2681 Thread_4248685   DispatchQueue_1: com.apple.main-thread  (serial)
    + 2681 start  (in libdyld.dylib) + 1  [0x7fff59edc3d5]
    +   2681 main  (in fish) + 7938  [0x100e187ae]
    +     2681 reader_read(parser_t&, int, io_chain_t const&)  (in fish) + 1406  [0x100ede5f9]
    +       2681 reader_data_t::readline(int)  (in fish) + 1124  [0x100edd722]
    +         2681 reader_data_t::read_normal_chars(readline_loop_state_t&)  (in fish) + 249  [0x100ed87cd]
    +           2681 inputter_t::readch(std::__1::function<void (std::__1::vector<std::__1::basic_string<wchar_t, std::__1::char_traits<wchar_t>, std::__1::allocator<wchar_t> >, std::__1::allocator<std::__1::basic_string<wchar_t, std::__1::char_traits<wchar_t>, std::__1::allocator<wchar_t> > > > const&)> const&)  (in fish) + 107  [0x100ea3d0f]
    +             2681 input_event_queue_t::readch()  (in fish) + 254  [0x100eaa15a]
    +               2681 input_event_queue_t::readb()  (in fish) + 357  [0x100ea9a93]
    +                 2681 __select  (in libsystem_kernel.dylib) + 10  [0x7fff5a018616]
    2681 Thread_4352150
      2681 thread_start  (in libsystem_pthread.dylib) + 13  [0x7fff5a0cf40d]
        2681 _pthread_start  (in libsystem_pthread.dylib) + 66  [0x7fff5a0d3249]
          2681 _pthread_body  (in libsystem_pthread.dylib) + 126  [0x7fff5a0d02eb]
            2681 thread_pool_t::run_trampoline(void*)  (in fish) + 14  [0x100eada56]
              2681 thread_pool_t::run()  (in fish) + 243  [0x100ead869]
                2681 std::__1::__function::__func<debounce_t::perform_impl(std::__1::function<void ()>, std::__1::function<void ()>)::$_0, std::__1::allocator<debounce_t::perform_impl(std::__1::function<void ()>, std::__1::function<void ()>)::$_0>, void ()>::operator()()  (in fish) + 22  [0x100eaf7d4]
                  2681 debounce_t::impl_t::run_next(unsigned long long)  (in fish) + 251  [0x100eae619]
                    2681 std::__1::__function::__func<iothread_trampoline_t<std::__1::function<(anonymous namespace)::highlight_result_t ()>, reader_data_t::super_highlight_me_plenty()::$_2, (anonymous namespace)::highlight_result_t>::iothread_trampoline_t(std::__1::function<(anonymous namespace)::highlight_result_t ()> const&, reader_data_t::super_highlight_me_plenty()::$_2 const&)::'lambda'(), std::__1::allocator<iothread_trampoline_t<std::__1::function<(anonymous namespace)::highlight_result_t ()>, reader_data_t::super_highlight_me_plenty()::$_2, (anonymous namespace)::highlight_result_t>::iothread_trampoline_t(std::__1::function<(anonymous namespace)::highlight_result_t ()> const&, reader_data_t::super_highlight_me_plenty()::$_2 const&)::'lambda'()>, void ()>::operator()()  (in fish) + 39  [0x100ee329f]
                      2681 std::__1::__function::__func<get_highlight_performer(parser_t&, std::__1::basic_string<wchar_t, std::__1::char_traits<wchar_t>, std::__1::allocator<wchar_t> > const&, bool)::$_8, std::__1::allocator<get_highlight_performer(parser_t&, std::__1::basic_string<wchar_t, std::__1::char_traits<wchar_t>, std::__1::allocator<wchar_t> > const&, bool)::$_8>, (anonymous namespace)::highlight_result_t ()>::operator()()  (in fish) + 241  [0x100ee0ec3]
                        2681 highlight_shell(std::__1::basic_string<wchar_t, std::__1::char_traits<wchar_t>, std::__1::allocator<wchar_t> > const&, std::__1::vector<highlight_spec_t, std::__1::allocator<highlight_spec_t> >&, operation_context_t const&, bool)  (in fish) + 180  [0x100e92c58]
                          2681 highlighter_t::highlight()  (in fish) + 624  [0x100e91400]
                            2681 is_potential_path(std::__1::basic_string<wchar_t, std::__1::char_traits<wchar_t>, std::__1::allocator<wchar_t> > const&, std::__1::vector<std::__1::basic_string<wchar_t, std::__1::char_traits<wchar_t>, std::__1::allocator<wchar_t> >, std::__1::allocator<std::__1::basic_string<wchar_t, std::__1::char_traits<wchar_t>, std::__1::allocator<wchar_t> > > > const&, operation_context_t const&, unsigned int)  (in fish) + 1540  [0x100e8f648]
                              2681 wdirname(std::__1::basic_string<wchar_t, std::__1::char_traits<wchar_t>, std::__1::allocator<wchar_t> > const&)  (in fish) + 57  [0x100ef5839]
                                2681 str2wcstring(char const*)  (in fish) + 21  [0x100e56b35]
                                  2681 _platform_strlen  (in libsystem_platform.dylib) + 18  [0x7fff5a0c46f2]

@zanchey zanchey added bug Something that's not working as intended and removed needs more info labels Mar 20, 2021
@ross2084
Copy link

ross2084 commented Mar 20, 2021

A bit more hopefully helpful debugging info is that the long text needs to be the 2nd or greater word in a command string. For example, pasting this pattern doesn't trigger the high CPU issue:

<long string> opt1 opt2

However this does trigger it, but only after pasting the second instance of "long string":

<long string> <long string>

@ridiculousfish
Copy link
Member

Ugh, this is bad. There's two issues: dirname is failing and SIGSEGV is being dropped; both are Mac/BSD specific.

Syntax highlighting tries to determine which arguments may be valid paths, so it can underline them. We pass the string to dirname to get its parent. I believed dirname merely trims the string from the last slash, which is correct on Linux: it cannot error. But on Mac dirname copies the string to internal storage, and will error if the string exceeds PATH_MAX, in which case it returns null.

So we try to construct a std::string from null and this produces a SIGSEGV. I had thought that if a thread triggered SIGSEGV, the signal would be delivered somewhere (e.g. to the main thread) or else the program would be terminated, and again on Linux this is correct. But on Mac the signal is just dropped; the thread then repeats and gets another SIGSEGV, leading to the infinite loop.

Fixes need to be:

  1. Don't block the four error signals (SIGBUS, SIGFPE, SIGILL, or SIGSEGV) in background threads. This would allow fish to crash on Mac.
  2. Stop using system provided dirname, it's a big footgun
  3. Optimization: skip path detection for arguments of length > PATH_MAX.

ridiculousfish added a commit that referenced this issue Mar 22, 2021
Previously fish attempted to block all signals on background threads, so
that they would be delivered to the main thread. But on Mac, SIGSEGV
and probably some others just get silently dropped, leading to potential
infinite loops instead of crashing. So stop blocking these signals.

With this change the null-deref in #7837 will properly crash instead of
spinning.
ridiculousfish added a commit that referenced this issue Mar 22, 2021
When fish performs syntax highlighting, it attempts to determine which
arguments are valid paths and underline them. Skip paths whose length
exceeds PATH_MAX. This is an optimization: such strings are almost
certainly not valid paths and checking them may be expensive.

Relevant is #7837
@IlanCosman
Copy link
Contributor

IlanCosman commented Mar 22, 2021

Now that we're shipping basename and dirname replacements, could we make those commands? I know it's something that has been requested a bunch.

@zanchey
Copy link
Member

zanchey commented Mar 22, 2021

Now that we're shipping basename and dirname replacements, could we make those commands? I know it's something that has been requested a bunch.

Has it? They're core utilities, so should be present everywhere. #7076 added shortcuts for the currently running script.

@faho faho added this to the fish 3.3.0 milestone Mar 29, 2021
@zanchey zanchey modified the milestones: fish 3.3.0, fish 3.2.2 Apr 1, 2021
zanchey pushed a commit that referenced this issue Apr 1, 2021
Previously fish attempted to block all signals on background threads, so
that they would be delivered to the main thread. But on Mac, SIGSEGV
and probably some others just get silently dropped, leading to potential
infinite loops instead of crashing. So stop blocking these signals.

With this change the null-deref in #7837 will properly crash instead of
spinning.

(cherry picked from commit a7c37e4)
zanchey pushed a commit that referenced this issue Apr 1, 2021
Previously wbasename and wdirname wrapped the system-provided basename
and dirname. But these have thread-safety issues and some surprising
error conditions on Mac. Just reimplement these per the OpenGroup spec.

In particular these no longer trigger a null-dereference if the input
exceeds PATH_MAX.

Add some tests too.

This fixes #7837

(cherry picked from commit cf35431)
zanchey pushed a commit that referenced this issue Apr 1, 2021
When fish performs syntax highlighting, it attempts to determine which
arguments are valid paths and underline them. Skip paths whose length
exceeds PATH_MAX. This is an optimization: such strings are almost
certainly not valid paths and checking them may be expensive.

Relevant is #7837

(cherry picked from commit 8d54d2b)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something that's not working as intended
Projects
None yet
Development

No branches or pull requests

8 participants