From 531a488fdd5c243abc034f8f6cc62f7c9e2f31ae Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Fri, 28 Jul 2023 12:56:54 -0400 Subject: [PATCH] fix: renderer race condition Guard accessing the underlying Termenv output behind a mutex. Multiple goroutines can set/get the color profile causing a race condition. Needs: https://github.com/muesli/termenv/pull/146 --- go.mod | 6 +++--- go.sum | 8 ++++++++ renderer.go | 16 ++++++++++++++-- renderer_test.go | 20 +++++++++++++++++++- style.go | 7 ++++--- style_test.go | 19 ++++++++++--------- 6 files changed, 58 insertions(+), 18 deletions(-) diff --git a/go.mod b/go.mod index a1376ba0..e757167f 100644 --- a/go.mod +++ b/go.mod @@ -5,13 +5,13 @@ go 1.17 require ( github.com/mattn/go-runewidth v0.0.14 github.com/muesli/reflow v0.3.0 - github.com/muesli/termenv v0.15.1 + github.com/muesli/termenv v0.15.3-0.20230728164039-3cf3563b77d7 ) require ( github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect github.com/lucasb-eyer/go-colorful v1.2.0 // indirect - github.com/mattn/go-isatty v0.0.17 // indirect + github.com/mattn/go-isatty v0.0.19 // indirect github.com/rivo/uniseg v0.2.0 // indirect - golang.org/x/sys v0.6.0 // indirect + golang.org/x/sys v0.10.0 // indirect ) diff --git a/go.sum b/go.sum index 2c273fd1..14af7911 100644 --- a/go.sum +++ b/go.sum @@ -4,6 +4,8 @@ github.com/lucasb-eyer/go-colorful v1.2.0 h1:1nnpGOrhyZZuNyfu1QjKiUICQ74+3FNCN69 github.com/lucasb-eyer/go-colorful v1.2.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= github.com/mattn/go-isatty v0.0.17 h1:BTarxUcIeDqL27Mc+vyvdWYSL28zpIhv3RoTdsLMPng= github.com/mattn/go-isatty v0.0.17/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= +github.com/mattn/go-isatty v0.0.19 h1:JITubQf0MOLdlGRuRq+jtsDlekdYPia9ZFsB8h/APPA= +github.com/mattn/go-isatty v0.0.19/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/mattn/go-runewidth v0.0.12/go.mod h1:RAqKPSqVFrSLVXbA8x7dzmKdmGzieGRCM46jaSJTDAk= github.com/mattn/go-runewidth v0.0.14 h1:+xnbZSEeDbOIg5/mE6JF0w6n9duR1l3/WmbinWVwUuU= github.com/mattn/go-runewidth v0.0.14/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w= @@ -11,9 +13,15 @@ github.com/muesli/reflow v0.3.0 h1:IFsN6K9NfGtjeggFP+68I4chLZV2yIKsXJFNZ+eWh6s= github.com/muesli/reflow v0.3.0/go.mod h1:pbwTDkVPibjO2kyvBQRBxTWEEGDGq0FlB1BIKtnHY/8= github.com/muesli/termenv v0.15.1 h1:UzuTb/+hhlBugQz28rpzey4ZuKcZ03MeKsoG7IJZIxs= github.com/muesli/termenv v0.15.1/go.mod h1:HeAQPTzpfs016yGtA4g00CsdYnVLJvxsS4ANqrZs2sQ= +github.com/muesli/termenv v0.15.3-0.20230728162136-de1ed946b031 h1:mSQxHDNYTlwIFgiPPz8W+rulsHVCFzFAbbc/XQo7BfI= +github.com/muesli/termenv v0.15.3-0.20230728162136-de1ed946b031/go.mod h1:cTIDIpWz9VkemD0+7lFZuZ8my3zF4iDi115tBmAocz0= +github.com/muesli/termenv v0.15.3-0.20230728164039-3cf3563b77d7 h1:FNxK5hfhxljbUS5MbiZs6iAyo6dMI2qLI+G8q0GJhVE= +github.com/muesli/termenv v0.15.3-0.20230728164039-3cf3563b77d7/go.mod h1:cTIDIpWz9VkemD0+7lFZuZ8my3zF4iDi115tBmAocz0= github.com/rivo/uniseg v0.1.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc= github.com/rivo/uniseg v0.2.0 h1:S1pD9weZBuJdFmowNwbpi7BJ8TNftyUImj/0WQi72jY= github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0 h1:MVltZSvRTcU2ljQOhs94SXPftV6DCNnZViHeQps87pQ= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.10.0 h1:SqMFp9UcQJZa+pmYuAKjd9xq1f0j5rLcDIk0mj4qAsA= +golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= diff --git a/renderer.go b/renderer.go index 4bea8374..660350d6 100644 --- a/renderer.go +++ b/renderer.go @@ -2,6 +2,7 @@ package lipgloss import ( "io" + "sync" "github.com/muesli/termenv" ) @@ -16,6 +17,7 @@ var renderer = &Renderer{ type Renderer struct { output *termenv.Output hasDarkBackground *bool + mtx sync.RWMutex } // RendererOption is a function that can be used to configure a [Renderer]. @@ -43,17 +45,21 @@ func NewRenderer(w io.Writer, opts ...termenv.OutputOption) *Renderer { // Output returns the termenv output. func (r *Renderer) Output() *termenv.Output { + r.mtx.RLock() + defer r.mtx.RUnlock() return r.output } // SetOutput sets the termenv output. func (r *Renderer) SetOutput(o *termenv.Output) { + r.mtx.Lock() + defer r.mtx.Unlock() r.output = o } // ColorProfile returns the detected termenv color profile. func (r *Renderer) ColorProfile() termenv.Profile { - return r.output.Profile + return r.output.ColorProfile() } // ColorProfile returns the detected termenv color profile. @@ -78,7 +84,9 @@ func ColorProfile() termenv.Profile { // // This function is thread-safe. func (r *Renderer) SetColorProfile(p termenv.Profile) { - r.output.Profile = p + r.mtx.Lock() + defer r.mtx.Unlock() + r.output.SetColorProfile(p) } // SetColorProfile sets the color profile on the default renderer. This @@ -110,6 +118,8 @@ func HasDarkBackground() bool { // background. A dark background can either be auto-detected, or set explicitly // on the renderer. func (r *Renderer) HasDarkBackground() bool { + r.mtx.RLock() + defer r.mtx.RUnlock() if r.hasDarkBackground != nil { return *r.hasDarkBackground } @@ -139,5 +149,7 @@ func SetHasDarkBackground(b bool) { // // This function is thread-safe. func (r *Renderer) SetHasDarkBackground(b bool) { + r.mtx.Lock() + defer r.mtx.Unlock() r.hasDarkBackground = &b } diff --git a/renderer_test.go b/renderer_test.go index 7f05acd7..5381c4b6 100644 --- a/renderer_test.go +++ b/renderer_test.go @@ -1,6 +1,7 @@ package lipgloss import ( + "io" "os" "testing" @@ -29,7 +30,24 @@ func TestRendererWithOutput(t *testing.T) { defer os.Remove(f.Name()) r := NewRenderer(f) r.SetColorProfile(termenv.TrueColor) - if r.output.Profile != termenv.TrueColor { + if r.output.ColorProfile() != termenv.TrueColor { t.Error("Expected renderer to use true color") } } + +func TestRace(t *testing.T) { + r := NewRenderer(io.Discard) + o := r.Output() + + for i := 0; i < 100; i++ { + t.Run("SetColorProfile", func(t *testing.T) { + t.Parallel() + r.SetHasDarkBackground(false) + r.HasDarkBackground() + r.SetOutput(o) + r.SetColorProfile(termenv.ANSI256) + r.SetHasDarkBackground(true) + r.Output() + }) + } +} diff --git a/style.go b/style.go index e94b8670..e7cd8c71 100644 --- a/style.go +++ b/style.go @@ -182,9 +182,10 @@ func (s Style) Render(strs ...string) string { var ( str = joinString(strs...) - te = s.r.ColorProfile().String() - teSpace = s.r.ColorProfile().String() - teWhitespace = s.r.ColorProfile().String() + p = s.r.ColorProfile() + te = p.String() + teSpace = p.String() + teWhitespace = p.String() bold = s.getAsBool(boldKey, false) italic = s.getAsBool(italicKey, false) diff --git a/style_test.go b/style_test.go index 5e1eb731..92a2ac15 100644 --- a/style_test.go +++ b/style_test.go @@ -9,8 +9,9 @@ import ( ) func TestStyleRender(t *testing.T) { - renderer.SetColorProfile(termenv.TrueColor) - renderer.SetHasDarkBackground(true) + r := NewRenderer(ioutil.Discard) + r.SetColorProfile(termenv.TrueColor) + r.SetHasDarkBackground(true) t.Parallel() tt := []struct { @@ -18,31 +19,31 @@ func TestStyleRender(t *testing.T) { expected string }{ { - NewStyle().Foreground(Color("#5A56E0")), + r.NewStyle().Foreground(Color("#5A56E0")), "\x1b[38;2;89;86;224mhello\x1b[0m", }, { - NewStyle().Foreground(AdaptiveColor{Light: "#fffe12", Dark: "#5A56E0"}), + r.NewStyle().Foreground(AdaptiveColor{Light: "#fffe12", Dark: "#5A56E0"}), "\x1b[38;2;89;86;224mhello\x1b[0m", }, { - NewStyle().Bold(true), + r.NewStyle().Bold(true), "\x1b[1mhello\x1b[0m", }, { - NewStyle().Italic(true), + r.NewStyle().Italic(true), "\x1b[3mhello\x1b[0m", }, { - NewStyle().Underline(true), + r.NewStyle().Underline(true), "\x1b[4;4mh\x1b[0m\x1b[4;4me\x1b[0m\x1b[4;4ml\x1b[0m\x1b[4;4ml\x1b[0m\x1b[4;4mo\x1b[0m", }, { - NewStyle().Blink(true), + r.NewStyle().Blink(true), "\x1b[5mhello\x1b[0m", }, { - NewStyle().Faint(true), + r.NewStyle().Faint(true), "\x1b[2mhello\x1b[0m", }, }