Data races about go-sse

[FatalReadError]

Unfortunately I can't right now, but I will try to dig up more information and possibly even a reproduction test case.

[alexandrevicenzi]

There are many things wrong, I'm working on a proper fix for the entire lib, but it will take a bit more time.

[FatalReadError]

These two data races seem rare and hard to trigger.

[alexandrevicenzi]

Could you share some of your code?

[codewinch]

Don't know if this is relevant, but I've experienced some of this:

WARNING: DATA RACE
Write at 0x00c000284018 by goroutine 13:
  github.com/alexandrevicenzi/go-sse.(*Channel).SendMessage()
      /home/cw/go/pkg/mod/github.com/alexandrevicenzi/[email protected]/channel.go:26 +0x8c
  github.com/alexandrevicenzi/go-sse.(*Server).SendMessage()
      /home/cw/go/pkg/mod/github.com/alexandrevicenzi/[email protected]/sse.go:119 +0x21b

I was able to fix it by just wrapping a Lock() and Unlock() around that c.lastEventID = message.id in channel.go:26 like this:

// SendMessage broadcast a message to all clients in a channel.                                                    
func (c *Channel) SendMessage(message *Message) {                                                                  
                                                                                                                   
    c.mu.Lock()      // <-- new                                                                                              
    c.lastEventID = message.id                                                                                     
    c.mu.Unlock()   // <-- new                                                                                               
                                                                                                                   
    c.mu.RLock()                                                                                                   
                                                                                                                   
    for c, open := range c.clients {                                                                               
        if open {                                                                                                  
            c.send <- message                                                                                      
        }                                                                                                          
    }                                                                                                              
                                                                                                                   
    c.mu.RUnlock()                                                                                                 
}  

At least in initial testing, it resolves the data race.

[codewinch]

In my case, it was very reproducible and this instantly fixed it.

[codewinch]

Spoke too soon -- have another on sse.go:96:

msg.retry = s.options.RetryInterval

[alexandrevicenzi]

Yes, it can happen anywhere, the lib is using goroutines where it's not needed and for that needs locks everywhere. The way to fix it is by removing most and leave only when required.

[iscander]

Can reproduce issue with set last lastEventID. My pattern of usage send multiple messages of different time to the same client like multiple topics subscriptions. It easy appears in tests, due absence of significant delay.

Race log is follow:

WARNING: DATA RACE
Write at 0x00c000436258 by goroutine 49:
github.com/alexandrevicenzi/go-sse.(*Channel).SendMessage()
/vendor/github.com/alexandrevicenzi/go-sse/channel.go:26 +0x8c
github.com/alexandrevicenzi/go-sse.(*Server).SendMessage()
/vendor/github.com/alexandrevicenzi/go-sse/sse.go:116 +0x21b
/rest/handlers.(*eventsHandler).trackFlow.func1()
/rest/handlers/events.go:137 +0x124

Previous write at 0x00c000436258 by goroutine 48:
github.com/alexandrevicenzi/go-sse.(*Channel).SendMessage()
/vendor/github.com/alexandrevicenzi/go-sse/channel.go:26 +0x8c
github.com/alexandrevicenzi/go-sse.(*Server).SendMessage()
/vendor/github.com/alexandrevicenzi/go-sse/sse.go:116 +0x21b
/rest/handlers.(*eventsHandler).trackFlow.func1()
/rest/handlers/events.go:137 +0x124

and the patch is exactly the same:

diff --git a/go/vendor/github.com/alexandrevicenzi/go-sse/channel.go b/go/vendor/github.com/alexandrevicenzi/go-sse/channel.go
index 82be4197..c66f948e 100644
--- a/go/vendor/github.com/alexandrevicenzi/go-sse/channel.go
+++ b/go/vendor/github.com/alexandrevicenzi/go-sse/channel.go
@@ -23,7 +23,9 @@ func newChannel(name string) *Channel {

 // SendMessage broadcast a message to all clients in a channel.
 func (c *Channel) SendMessage(message *Message) {
+       c.mu.Lock()
        c.lastEventID = message.id
+       c.mu.Unlock()

        c.mu.RLock()

[iscander]

https://gist.github.com/iscander/9a144528e563dc5969346d5e0b61cd4f yet another sample which rises same data race but as a part of HTTPHandler