search

charmbracelet / lipgloss

Style definitions for nice terminal layouts 👄
MIT License
8.25k stars 230 forks source link

Long pause/halt when lipgloss is determining background color #73

Open jefferai opened 2 years ago

jefferai commented 2 years ago

Sometimes when running my program (very similar to the fancy list demo and using the same lipgloss variables), when my list is created, I see a very long pause. It doesn't happen all the time -- over half the time, but not every time -- but that pause can take anywhere from a few seconds to 30 seconds or more. I got a SIGQUIT stack trace -- it's down in termenv so let me know if you'd rather I post there than here, but since my entrypoint there is lipgloss I figured I'd start here.

goroutine 1 [syscall]:
syscall.syscall(0x10060cbe0, 0x1, 0x14000390077, 0x1)
        /opt/homebrew/Cellar/go/1.17.8/libexec/src/runtime/sys_darwin.go:22 +0x1c
syscall.read(0x1, {0x14000390077, 0x1, 0x1})
        /opt/homebrew/Cellar/go/1.17.8/libexec/src/syscall/zsyscall_darwin_arm64.go:1171 +0x5c
syscall.Read(...)
        /opt/homebrew/Cellar/go/1.17.8/libexec/src/syscall/syscall_unix.go:189
internal/poll.ignoringEINTRIO(...)
        /opt/homebrew/Cellar/go/1.17.8/libexec/src/internal/poll/fd_unix.go:582
internal/poll.(*FD).Read(0x140001a0060, {0x14000390077, 0x1, 0x1})
        /opt/homebrew/Cellar/go/1.17.8/libexec/src/internal/poll/fd_unix.go:163 +0x214
os.(*File).read(...)
        /opt/homebrew/Cellar/go/1.17.8/libexec/src/os/file_posix.go:32
os.(*File).Read(0x1400019e008, {0x14000390077, 0x1, 0x1})
        /opt/homebrew/Cellar/go/1.17.8/libexec/src/os/file.go:119 +0x74
github.com/muesli/termenv.readNextByte(0x1400019e008)
        /Users/jeff/go/pkg/mod/github.com/muesli/termenv@v0.11.1-0.20220212125758-44cd13922739/termenv_unix.go:135 +0x98
github.com/muesli/termenv.readNextResponse(0x1400019e008)
        /Users/jeff/go/pkg/mod/github.com/muesli/termenv@v0.11.1-0.20220212125758-44cd13922739/termenv_unix.go:185 +0x1c8
github.com/muesli/termenv.termStatusReport(0xb)
        /Users/jeff/go/pkg/mod/github.com/muesli/termenv@v0.11.1-0.20220212125758-44cd13922739/termenv_unix.go:246 +0x22c
github.com/muesli/termenv.backgroundColor()
        /Users/jeff/go/pkg/mod/github.com/muesli/termenv@v0.11.1-0.20220212125758-44cd13922739/termenv_unix.go:85 +0x28
github.com/muesli/termenv.BackgroundColor()
        /Users/jeff/go/pkg/mod/github.com/muesli/termenv@v0.11.1-0.20220212125758-44cd13922739/termenv.go:67 +0x6c
github.com/muesli/termenv.HasDarkBackground()
        /Users/jeff/go/pkg/mod/github.com/muesli/termenv@v0.11.1-0.20220212125758-44cd13922739/termenv.go:72 +0x20
github.com/charmbracelet/lipgloss.HasDarkBackground.func1()
        /Users/jeff/go/pkg/mod/github.com/charmbracelet/lipgloss@v0.5.0/color.go:60 +0x20
sync.(*Once).doSlow(0x1070a68a8, 0x10653e450)
        /opt/homebrew/Cellar/go/1.17.8/libexec/src/sync/once.go:68 +0x10c
sync.(*Once).Do(...)
        /opt/homebrew/Cellar/go/1.17.8/libexec/src/sync/once.go:59
github.com/charmbracelet/lipgloss.HasDarkBackground()
        /Users/jeff/go/pkg/mod/github.com/charmbracelet/lipgloss@v0.5.0/color.go:59 +0x5c
github.com/charmbracelet/lipgloss.AdaptiveColor.value(...)
        /Users/jeff/go/pkg/mod/github.com/charmbracelet/lipgloss@v0.5.0/color.go:167
github.com/charmbracelet/lipgloss.AdaptiveColor.color({{0x101437372, 0x7}, {0x101437491, 0x7}})
        /Users/jeff/go/pkg/mod/github.com/charmbracelet/lipgloss@v0.5.0/color.go:174 +0x2c
github.com/charmbracelet/lipgloss.Style.Render({0x140006749f0, {0x101432dc9, 0x3}}, {0x101432dc9, 0x3})
        /Users/jeff/go/pkg/mod/github.com/charmbracelet/lipgloss@v0.5.0/style.go:230 +0x10e0
github.com/charmbracelet/lipgloss.Style.String(...)
        /Users/jeff/go/pkg/mod/github.com/charmbracelet/lipgloss@v0.5.0/style.go:109
github.com/charmbracelet/bubbles/list.New({0x0, 0x0, 0x0}, {0x106585f38, 0x140000dea90}, 0x0, 0x0)
        /Users/jeff/go/pkg/mod/github.com/charmbracelet/bubbles@v0.10.3/list/list.go:165 +0x2a4
<my code>

Let me know what other info I can provide. Thanks!

muesli commented 2 years ago

Hey @jefferai, long time no see!

termenv is querying the terminal for its current settings via control sequences on stdout, and the terminal is expected to respond by printing to stdout, which termenv reads from. There's a timeout of 5 seconds defined, after which the read should be aborted: https://github.com/muesli/termenv/blob/master/termenv_unix.go#L112

It looks like waitForData is reporting data to be available, but then the read call blocks waiting for a single byte. I haven't seen that on any of my (test-) systems so far.

Could you verify whether bytes are actually slowly trickling in or if it's stuck there waiting for a response that'll never arrive?

jefferai commented 2 years ago

Hi @muesli 😀

It's definitely not 5 seconds. Sometimes it's 2 or 3; often it's 10 or so; sometimes (very rarely) it's 30 or more. I don't think I've seen it completely freeze forever.

How would you want me to verify that bytes are trickling in? Just fork the lib and print debug out?

I can also see if it happens in other envs. I'm running on arm64 M1 on Monterey 12.2.1 in iTerm using zsh. I did try with bash without any difference. Stock terminal makes no difference.

ahmedxfn commented 2 years ago

Same exact issue here, this issue appeared when I tried the component Help from https://github.com/charmbracelet/bubbles, after some investigating, I figured out that when removing lipgloss.JoinHorizontal from this line and this line, plus removing the renderer from here, it works without freezing.

ahmedxfn commented 2 years ago

After more investigating, the real cause for me was from lipgloss.AdaptiveColor which call termenv.HasDarkBackground() to determine the background color as @jefferai stated, replacing the adaptive color with normal color solved the issue.

muesli commented 2 years ago

@r4ndomx That observation is correct, yeah. I think I finally figured out under which circumstances this is happening: if the first background detection happens only after bubbletea has taken over control of stdin/out, the communication channel between terminal and termenv is essentially broken. You could work around this issue by manually calling lipgloss.SetHasDarkBackground(termenv.HasDarkBackground()) once before initializing your bubbletea app.

We'll clean this up and may have to refactor the API a bit here. See upcoming work in next branch.

DotoPototo commented 6 months ago

Is there any update on having this issue fixed within lipgloss itself?

My understanding is that the issue arises because Bubbletea holds a lock on the os.Stdout file descriptor, which prevents lipgloss from performing terminal background color checks required for adaptive colors.

Calling lipgloss.SetHasDarkBackground(termenv.HasDarkBackground()) does indeed fix the issue. A potential alternative solution might involve using a shared mutex to synchronize access to the file descriptor, but this would only be needed if the background color was changing (though I'm sure there's also some caching involved that would need to be updated too).

In the meantime, I think it would be helpful to update the README to mention this issue under the Adaptive Colors section. This would save other developers a lot of time in figuring out the problem.

Thank you!

meowgorithm commented 6 months ago

@mikecbone Noted! For what it's worth we have some upcoming features that both run background color detection in a non-blocking way and let you opt out of it entirely.

ElectricPulse commented 1 week ago

I am still having this issue as of version v0.12.1 when using list.New on fedora 41.

meowgorithm commented 6 days ago

@ElectricPulse How about with v1.0.0?

If you're still having that issue as of that version, can you post some source code we can use to reproduce the issue?