Skip to content

Commit

Permalink
fix!: output race condition
Browse files Browse the repository at this point in the history
Use a mutex to guard setting/getting the color profile and fg/bg colors.
Use `SetColorProfile` to change the output color profile.
Use pointer receiver since we have a lock in the struct.

Fixes: charmbracelet/lipgloss#210
Fixes: #145
  • Loading branch information
aymanbagabas committed Jul 28, 2023
1 parent 7165365 commit 27addc5
Show file tree
Hide file tree
Showing 10 changed files with 118 additions and 70 deletions.
4 changes: 2 additions & 2 deletions copy.go
Expand Up @@ -7,7 +7,7 @@ import (
)

// Copy copies text to clipboard using OSC 52 escape sequence.
func (o Output) Copy(str string) {
func (o *Output) Copy(str string) {
s := osc52.New(str)
if strings.HasPrefix(o.environ.Getenv("TERM"), "screen") {
s = s.Screen()
Expand All @@ -17,7 +17,7 @@ func (o Output) Copy(str string) {

// CopyPrimary copies text to primary clipboard (X11) using OSC 52 escape
// sequence.
func (o Output) CopyPrimary(str string) {
func (o *Output) CopyPrimary(str string) {
s := osc52.New(str).Primary()
if strings.HasPrefix(o.environ.Getenv("TERM"), "screen") {
s = s.Screen()
Expand Down
37 changes: 28 additions & 9 deletions output.go
Expand Up @@ -22,7 +22,7 @@ type OutputOption = func(*Output)

// Output is a terminal output.
type Output struct {
Profile
profile Profile
tty io.Writer
environ Environ

Expand All @@ -33,6 +33,8 @@ type Output struct {
fgColor Color
bgSync *sync.Once
bgColor Color

mtx sync.RWMutex
}

// Environ is an interface for getting environment variables.
Expand Down Expand Up @@ -66,7 +68,7 @@ func NewOutput(tty io.Writer, opts ...OutputOption) *Output {
o := &Output{
tty: tty,
environ: &osEnviron{},
Profile: -1,
profile: -1,
fgSync: &sync.Once{},
fgColor: NoColor{},
bgSync: &sync.Once{},
Expand All @@ -79,8 +81,8 @@ func NewOutput(tty io.Writer, opts ...OutputOption) *Output {
for _, opt := range opts {
opt(o)
}
if o.Profile < 0 {
o.Profile = o.EnvColorProfile()
if o.profile < 0 {
o.profile = o.EnvColorProfile()
}

return o
Expand All @@ -96,7 +98,7 @@ func WithEnvironment(environ Environ) OutputOption {
// WithProfile returns a new OutputOption for the given profile.
func WithProfile(profile Profile) OutputOption {
return func(o *Output) {
o.Profile = profile
o.profile = profile
}
}

Expand Down Expand Up @@ -135,7 +137,16 @@ func WithUnsafe() OutputOption {

// ColorProfile returns the supported color profile:

Check failure on line 138 in output.go

View workflow job for this annotation

GitHub Actions / lint-soft

Comment should end in a period (godot)
func (o Output) ColorProfile() Profile {

Check failure on line 139 in output.go

View workflow job for this annotation

GitHub Actions / lint

copylocks: ColorProfile passes lock by value: github.com/muesli/termenv.Output contains sync.RWMutex (govet)
return o.Profile
o.mtx.RLock()
defer o.mtx.RUnlock()
return o.profile
}

// SetColorProfile sets the color profile.
func (o *Output) SetColorProfile(profile Profile) {
o.mtx.Lock()
defer o.mtx.Unlock()
o.profile = profile
}

// ForegroundColor returns the terminal's default foreground color.
Expand All @@ -145,7 +156,9 @@ func (o *Output) ForegroundColor() Color {
return
}

o.mtx.Lock()
o.fgColor = o.foregroundColor()
o.mtx.Unlock()
}

if o.cache {
Expand All @@ -154,6 +167,8 @@ func (o *Output) ForegroundColor() Color {
f()
}

o.mtx.RLock()
defer o.mtx.RUnlock()
return o.fgColor
}

Expand All @@ -164,7 +179,9 @@ func (o *Output) BackgroundColor() Color {
return
}

o.mtx.Lock()
o.bgColor = o.backgroundColor()
o.mtx.Unlock()
}

if o.cache {
Expand All @@ -173,6 +190,8 @@ func (o *Output) BackgroundColor() Color {
f()
}

o.mtx.RLock()
defer o.mtx.RUnlock()
return o.bgColor
}

Expand All @@ -185,18 +204,18 @@ func (o *Output) HasDarkBackground() bool {

// TTY returns the terminal's file descriptor. This may be nil if the output is
// not a terminal.
func (o Output) TTY() File {
func (o *Output) TTY() File {
if f, ok := o.tty.(File); ok {
return f
}
return nil
}

func (o Output) Write(p []byte) (int, error) {
func (o *Output) Write(p []byte) (int, error) {
return o.tty.Write(p)
}

// WriteString writes the given string to the output.
func (o Output) WriteString(s string) (int, error) {
func (o *Output) WriteString(s string) (int, error) {
return o.Write([]byte(s))
}
22 changes: 22 additions & 0 deletions output_test.go
@@ -0,0 +1,22 @@
package termenv

import (
"io"
"testing"
)

func TestOutputRace(t *testing.T) {
o := NewOutput(io.Discard)
for i := 0; i < 100; i++ {
t.Run("Test race", func(t *testing.T) {
t.Parallel()
o.Write([]byte("test"))
o.SetColorProfile(ANSI)
o.ColorProfile()
o.HasDarkBackground()
o.TTY()
o.ForegroundColor()
o.BackgroundColor()
})
}
}

0 comments on commit 27addc5

Please sign in to comment.