Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: concurrent mutations to prefixWriter #1974

Merged

Conversation

GrahamDennis
Copy link
Contributor

When using the prefixed output format, I occasionally get a fatal error: concurrent map writes error due to concurrent modifications to the seen map used by Prefixed.

This change adds a mutex protecting the Prefixed struct.

Copy link
Member

@pd93 pd93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GrahamDennis Thanks for opening this. Could you test the following change for me?

diff --git a/internal/output/prefixed.go b/internal/output/prefixed.go
index 68d94156..fd2a2305 100644
--- a/internal/output/prefixed.go
+++ b/internal/output/prefixed.go
@@ -15,22 +15,21 @@ type Prefixed struct {
        logger  *logger.Logger
        seen    map[string]uint
        counter *uint
-       mutex   *sync.Mutex
+       mutex   sync.Mutex
 }
 
-func NewPrefixed(logger *logger.Logger) Prefixed {
+func NewPrefixed(logger *logger.Logger) *Prefixed {
        var counter uint
 
-       return Prefixed{
+       return &Prefixed{
                seen:    make(map[string]uint),
                counter: &counter,
                logger:  logger,
-               mutex:   &sync.Mutex{},
        }
 }
 
-func (p Prefixed) WrapWriter(stdOut, _ io.Writer, prefix string, _ *templater.Cache) (io.Writer, io.Writer, CloseFunc) {
-       pw := &prefixWriter{writer: stdOut, prefix: prefix, prefixed: &p}
+func (p *Prefixed) WrapWriter(stdOut, _ io.Writer, prefix string, _ *templater.Cache) (io.Writer, io.Writer, CloseFunc) {
+       pw := &prefixWriter{writer: stdOut, prefix: prefix, prefixed: p}
        return pw, pw, func(error) error { return pw.close() }
 }

No reason to use a pointer to a mutex here in my opinion.

@pd93 pd93 merged commit 0409c3c into go-task:main Dec 30, 2024
14 checks passed
@pd93
Copy link
Member

pd93 commented Dec 30, 2024

Added the suggested changes in 8ce9bdc. Thanks @GrahamDennis!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fatal error: concurrent map writes
3 participants