wagslane / go-rabbitmq

A wrapper of streadway/amqp that provides reconnection logic and sane defaults
https://blog.boot.dev/golang/connecting-to-rabbitmq-in-golang-easy/
MIT License
772 stars 126 forks source link

data race when calling Publisher.Close() #71

Closed rakete closed 2 years ago

rakete commented 2 years ago

I thought its my code since I store the publisher in a global var and then access it from goroutines, assuming that would be safe, and now I am having second thoughts about that assumption. But your example code for demonstrating how to use the Publisher has the same problem.

This code:

package main

import (
    "log"

    "github.com/wagslane/go-rabbitmq"
)

func main() {
    publisher, err := rabbitmq.NewPublisher(
        "amqp://user:password@localhost",
        rabbitmq.Config{},
        // can pass nothing for no logging
        rabbitmq.WithPublisherOptionsLogging,
    )
    if err != nil {
        log.Fatal(err)
    }
    // do this after err check or publisher might be nil
    defer publisher.Close()

    err = publisher.Publish(
        []byte("hello, world"),
        []string{"routing_key"},
        rabbitmq.WithPublishOptionsContentType("application/json"),
        rabbitmq.WithPublishOptionsMandatory,
        rabbitmq.WithPublishOptionsPersistentDelivery,
    )
    if err != nil {
        log.Fatal(err)
    }

    returns := publisher.NotifyReturn()
    go func() {
        for r := range returns {
            log.Printf("message returned from server: %s", string(r.Body))
        }
    }()
}

compiled with:

go build -race

produces this output:

~/g/s/g/r/dataRace> go build -race && ./dataRace
2022/04/14 17:08:50 message returned from server: hello, world
==================
WARNING: DATA RACE
Read at 0x00c000216068 by main goroutine:
  main.main()
      /home/rakete/go/src/github.com/rakete/dataRace/main.go:37 +0x3b7

Previous write at 0x00c000216068 by goroutine 13:
  github.com/wagslane/go-rabbitmq.(*Publisher).startNotifyFlowHandler()
      /home/rakete/go/pkg/mod/github.com/wagslane/go-rabbitmq@v0.8.0/publish.go:208 +0xee

Goroutine 13 (running) created at:
  github.com/wagslane/go-rabbitmq.NewPublisher()
      /home/rakete/go/pkg/mod/github.com/wagslane/go-rabbitmq@v0.8.0/publish.go:111 +0x492
  main.main()
      /home/rakete/go/src/github.com/rakete/dataRace/main.go:10 +0xfd
==================
2022/04/14 17:08:50 gorabbit: closing publisher...
2022/04/14 17:08:50 gorabbit: amqp channel closed gracefully
Found 1 data race(s)

The data race is detected almost every time, but sometimes it works fine. Repeat running it a couple of times to see the problem.

metalrex100 commented 2 years ago

@rakete you will remove race if you will wrap publisher.Close() with additional callback like this:

defer func() {
    publisher.Close()
}()

Despite such workaround fix race however I can't say why defer publisher.Close() produce race and additional callback makes things ok.

wagslane commented 2 years ago

I actually think this is a problem with the race detector: https://github.com/golang/go/issues/51459

I don't see any actual race issues. Feel free to open a PR if I'm missing something