Skip to content

Commit

Permalink
fix!: output race condition
Browse files Browse the repository at this point in the history
`ColorProfile` reads the terminal environment every time the function is
called. This is inefficient. We only need to read the `$TERM`
environment variable once when we initialize the output. So instead, we cache
the value we read.

Rename `ColorProfile` to `termColorProfile` and rely on
`EnvColorProfile` to detect the profile. Ideally, we would rely on the
terminal Terminfo to detect the profile and fallback to the environment.
But that's for another day :)

Make output profile accessible through `ColorProfile`.
Use `SetColorProfile` to change the output color profile.
Use a mutex to guard output writes.
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 3466887 commit 3c898b2
Show file tree
Hide file tree
Showing 10 changed files with 131 additions and 77 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
41 changes: 33 additions & 8 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 @@ -133,14 +135,31 @@ func WithUnsafe() OutputOption {
}
}

// ColorProfile returns the color profile.
// Ascii, ANSI, ANSI256, or TrueColor.
func (o *Output) ColorProfile() 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.
func (o *Output) ForegroundColor() Color {
f := func() {
if !o.isTTY() {
return
}

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

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

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

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

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

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

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

Expand All @@ -180,18 +205,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 3c898b2

Please sign in to comment.