Fix the data races and the test timeout that fail make test (closes #104)
check / check (push) Successful in 3s
check / check (push) Successful in 3s
The HTTP server built its router inside the goroutine that starts serving, so the start hook returned before the router existed, and the internal/handlers tests raced with it or hit a nil router. The start hook now runs configure, enableSentry and SetupRoutes, in that order, then serves in the background. The goroutine that sends queued messages to an IRC client read c.nick without c.mu while NICK changed it. Every such read now takes the lock. Under -race in the Docker build, internal/handlers takes over 30s on database work, not clock waits, so both go test runs in make test use -timeout 120s. The || retry stays (#101). Model: opus-5-5
This commit was merged in pull request #105.
This commit is contained in:
@@ -32,7 +32,7 @@ fmt-check:
|
|||||||
@test -z "$$(gofmt -l .)" || (echo "Files not formatted:" && gofmt -l . && exit 1)
|
@test -z "$$(gofmt -l .)" || (echo "Files not formatted:" && gofmt -l . && exit 1)
|
||||||
|
|
||||||
test: ensure-web-dist
|
test: ensure-web-dist
|
||||||
go test -timeout 30s -race -cover ./... || go test -timeout 30s -race -v ./...
|
go test -timeout 120s -race -cover ./... || go test -timeout 120s -race -v ./...
|
||||||
|
|
||||||
# check runs all validation without making changes
|
# check runs all validation without making changes
|
||||||
# Used by CI and Docker build — fails if anything is wrong
|
# Used by CI and Docker build — fails if anything is wrong
|
||||||
|
|||||||
@@ -128,7 +128,11 @@ func (c *Conn) deliverIRCMessage(
|
|||||||
default:
|
default:
|
||||||
// Unknown command — deliver as server notice.
|
// Unknown command — deliver as server notice.
|
||||||
if text != "" {
|
if text != "" {
|
||||||
c.sendFromServer("NOTICE", c.nick, text)
|
c.mu.Lock()
|
||||||
|
nick := c.nick
|
||||||
|
c.mu.Unlock()
|
||||||
|
|
||||||
|
c.sendFromServer("NOTICE", nick, text)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -158,8 +162,12 @@ func (c *Conn) deliverNumeric(
|
|||||||
_ = json.Unmarshal(msg.Params, ¶ms)
|
_ = json.Unmarshal(msg.Params, ¶ms)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
c.mu.Lock()
|
||||||
|
nick := c.nick
|
||||||
|
c.mu.Unlock()
|
||||||
|
|
||||||
allParams := make([]string, 0, 1+len(params)+1)
|
allParams := make([]string, 0, 1+len(params)+1)
|
||||||
allParams = append(allParams, c.nick)
|
allParams = append(allParams, nick)
|
||||||
allParams = append(allParams, params...)
|
allParams = append(allParams, params...)
|
||||||
|
|
||||||
if text != "" {
|
if text != "" {
|
||||||
@@ -177,8 +185,12 @@ func (c *Conn) deliverTextMessage(
|
|||||||
from := msg.From
|
from := msg.From
|
||||||
target := msg.To
|
target := msg.To
|
||||||
|
|
||||||
|
c.mu.Lock()
|
||||||
|
nick := c.nick
|
||||||
|
c.mu.Unlock()
|
||||||
|
|
||||||
// Don't echo our own messages back.
|
// Don't echo our own messages back.
|
||||||
if strings.EqualFold(from, c.nick) {
|
if strings.EqualFold(from, nick) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -192,9 +204,13 @@ func (c *Conn) deliverTextMessage(
|
|||||||
|
|
||||||
// deliverJoin sends a JOIN notification.
|
// deliverJoin sends a JOIN notification.
|
||||||
func (c *Conn) deliverJoin(msg *db.IRCMessage) {
|
func (c *Conn) deliverJoin(msg *db.IRCMessage) {
|
||||||
|
c.mu.Lock()
|
||||||
|
nick := c.nick
|
||||||
|
c.mu.Unlock()
|
||||||
|
|
||||||
// Don't echo our own JOINs (we already sent them
|
// Don't echo our own JOINs (we already sent them
|
||||||
// during joinChannel).
|
// during joinChannel).
|
||||||
if strings.EqualFold(msg.From, c.nick) {
|
if strings.EqualFold(msg.From, nick) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -206,7 +222,11 @@ func (c *Conn) deliverJoin(msg *db.IRCMessage) {
|
|||||||
|
|
||||||
// deliverPart sends a PART notification.
|
// deliverPart sends a PART notification.
|
||||||
func (c *Conn) deliverPart(msg *db.IRCMessage, text string) {
|
func (c *Conn) deliverPart(msg *db.IRCMessage, text string) {
|
||||||
if strings.EqualFold(msg.From, c.nick) {
|
c.mu.Lock()
|
||||||
|
nick := c.nick
|
||||||
|
c.mu.Unlock()
|
||||||
|
|
||||||
|
if strings.EqualFold(msg.From, nick) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -227,7 +247,11 @@ func (c *Conn) deliverNickChange(
|
|||||||
msg *db.IRCMessage,
|
msg *db.IRCMessage,
|
||||||
newNick string,
|
newNick string,
|
||||||
) {
|
) {
|
||||||
if strings.EqualFold(msg.From, c.nick) {
|
c.mu.Lock()
|
||||||
|
nick := c.nick
|
||||||
|
c.mu.Unlock()
|
||||||
|
|
||||||
|
if strings.EqualFold(msg.From, nick) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -241,7 +265,11 @@ func (c *Conn) deliverQuitMsg(
|
|||||||
msg *db.IRCMessage,
|
msg *db.IRCMessage,
|
||||||
text string,
|
text string,
|
||||||
) {
|
) {
|
||||||
if strings.EqualFold(msg.From, c.nick) {
|
c.mu.Lock()
|
||||||
|
nick := c.nick
|
||||||
|
c.mu.Unlock()
|
||||||
|
|
||||||
|
if strings.EqualFold(msg.From, nick) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -302,7 +330,11 @@ func (c *Conn) deliverInviteMsg(
|
|||||||
_ *db.IRCMessage,
|
_ *db.IRCMessage,
|
||||||
text string,
|
text string,
|
||||||
) {
|
) {
|
||||||
c.sendFromServer("NOTICE", c.nick, text)
|
c.mu.Lock()
|
||||||
|
nick := c.nick
|
||||||
|
c.mu.Unlock()
|
||||||
|
|
||||||
|
c.sendFromServer("NOTICE", nick, text)
|
||||||
}
|
}
|
||||||
|
|
||||||
// deliverMode sends a MODE change notification.
|
// deliverMode sends a MODE change notification.
|
||||||
|
|||||||
@@ -71,7 +71,14 @@ func New(
|
|||||||
lifecycle.Append(fx.Hook{
|
lifecycle.Append(fx.Hook{
|
||||||
OnStart: func(_ context.Context) error {
|
OnStart: func(_ context.Context) error {
|
||||||
srv.startupTime = time.Now()
|
srv.startupTime = time.Now()
|
||||||
go srv.Run() //nolint:contextcheck
|
|
||||||
|
// Build the router before OnStart returns, so srv can
|
||||||
|
// handle requests as soon as the app has started.
|
||||||
|
srv.configure()
|
||||||
|
srv.enableSentry()
|
||||||
|
srv.SetupRoutes()
|
||||||
|
|
||||||
|
go srv.serve() //nolint:contextcheck
|
||||||
|
|
||||||
return nil
|
return nil
|
||||||
},
|
},
|
||||||
@@ -83,13 +90,6 @@ func New(
|
|||||||
return srv, nil
|
return srv, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// Run starts the server configuration, Sentry, and begins serving.
|
|
||||||
func (srv *Server) Run() {
|
|
||||||
srv.configure()
|
|
||||||
srv.enableSentry()
|
|
||||||
srv.serve()
|
|
||||||
}
|
|
||||||
|
|
||||||
// ServeHTTP delegates to the chi router.
|
// ServeHTTP delegates to the chi router.
|
||||||
func (srv *Server) ServeHTTP(
|
func (srv *Server) ServeHTTP(
|
||||||
writer http.ResponseWriter,
|
writer http.ResponseWriter,
|
||||||
@@ -202,8 +202,6 @@ func (srv *Server) serveUntilShutdown() {
|
|||||||
Handler: srv,
|
Handler: srv,
|
||||||
}
|
}
|
||||||
|
|
||||||
srv.SetupRoutes()
|
|
||||||
|
|
||||||
srv.log.Info(
|
srv.log.Info(
|
||||||
"http begin listen", "listenaddr", listenAddr,
|
"http begin listen", "listenaddr", listenAddr,
|
||||||
)
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user