From 4100381a1f16c16fa891e06e62d94f5deaae4dd7 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 dark background color causing a race condition. Needs: https://github.com/muesli/termenv/pull/146 --- renderer.go | 12 ++++++++++++ renderer_test.go | 20 +++++++++++++++++++- style.go | 7 ++++--- style_test.go | 19 ++++++++++--------- 4 files changed, 45 insertions(+), 13 deletions(-) diff --git a/renderer.go b/renderer.go index 4bea8374..8e551f93 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,11 +45,15 @@ 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 } @@ -78,6 +84,8 @@ func ColorProfile() termenv.Profile { // // This function is thread-safe. func (r *Renderer) SetColorProfile(p termenv.Profile) { + r.mtx.Lock() + defer r.mtx.Unlock() r.output.Profile = p } @@ -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..6c0b145f 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.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 608c5970..ee50bcf7 100644 --- a/style.go +++ b/style.go @@ -185,9 +185,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 fd9c8aac..cecf84f4 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", }, }