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

memory leak #56

Closed QbCheng closed 2 years ago

QbCheng commented 2 years ago

https://github.com/wagslane/go-rabbitmq/blob/26cc90b71a689118e2c937dbde031a877fe9ead2/channel.go#L56

notifyCloseChan := chManager.channel.NotifyClose(make(chan *amqp.Error, 1)) notifyCancelChan := chManager.channel.NotifyCancel(make(chan string, 1))

The NotifyClose and NotifyCancel methods both add a monitor to the current channel and return to the monitor channel.

select {
        case err := <-notifyCloseChan:
           ...
        case err := <-notifyCancelChan:
          ...
}

If any channel returns an exception, you have performed NotifyClose and NotifyCancel on both channels, and the original channel will remain in AQMP, which should be considered a risk of memory leakage. If case err := <-notifyCancelChan is triggered all the time, there will be more and more closed listening notification channels saved in AQMP, and all you need is actually one

wagslane commented 2 years ago

Can you provide an example of this happening? After re-reading the docs, it seems we should be okay because we are closing the AMQP channels on reconnect.

QbCheng commented 2 years ago

I may be wrong. But I have a new problem, Use go build-race After compiling.

Read at 0x00c0005e81e8 by goroutine 48: github.com/rabbitmq/amqp091-go.(Channel).PublishWithDeferredConfirm() /root/go/pkg/mod/github.com/rabbitmq/amqp091-go@v1.3.0/channel.go:1376 +0x77d github.com/rabbitmq/amqp091-go.(Channel).Publish() /root/go/pkg/mod/github.com/rabbitmq/amqp091-go@v1.3.0/channel.go:1333 +0x677 github.com/wagslane/go-rabbitmq.(Publisher).Publish() /root/go/pkg/mod/github.com/wagslane/go-rabbitmq@v0.7.1/publish.go:173 +0x517 g1/util/router/bus.(ImplRabbitMQ).coroutinesSend() /mnt/hgfs/huaxia-server/util/router/bus/bus_impl_rmq.go:211 +0x4a7

Previous write at 0x00c0005e81e8 by goroutine 49: github.com/rabbitmq/amqp091-go.(Channel).Confirm() /root/go/pkg/mod/github.com/rabbitmq/amqp091-go@v1.3.0/channel.go:1532 +0x13c github.com/wagslane/go-rabbitmq.(Publisher).startNotifyPublishHandler() /root/go/pkg/mod/github.com/wagslane/go-rabbitmq@v0.7.1/publish.go:226 +0x71

The main reason

// NotifyPublish registers a listener for publish confirmations, must set ConfirmPublishings option func (publisher *Publisher) NotifyPublish() <-chan Confirmation { publisher.notifyPublishChan = make(chan Confirmation) go publisher.startNotifyPublishHandler() <--- You shouldn't open coroutines to do that return publisher.notifyPublishChan }

wagslane commented 2 years ago

Fixed in last push