pika/pika — Issue #97: Callbacks object appears to hold onto Channel instances after closing
Author: daf
Created: 2011-10-18 19:39:47+00:00
State: open

Hi,

For work we've developed a rather complex stack built on top of Pika for AMQP transport. I started tripping a bug that manifests similar to both #84 and #68 - the dreaded `ctag0` issue. In our case we are using a `SelectConnection` interface inside of a gevent based library. We're running against Pika 0.9.5.

The `ctag0` issue appears to have been fixed via the changes in 953d3d8028e6781b0978f6769fc9f8c92c7361ae, the specific problem being it was trying to put a delivered message in `Channel._on_basic_deliver` into the `self._pending` dictionary which didn't have the consumer tag anymore. However, I don't think the commit actually addresses the real issue: it appears that old instances of Channel are still referenced in the CallbackManager object.

If I obtain an instance of a Channel, use it, close it, then get the same numbered Channel again on my next request, it was triggering this bug. I rigged up a dump of the CallbackManager's `_callbacks` dict and was seeing that after every request, the list of callbacks registered for things like `_on_basic_deliver` and `Channel.close` etc still had a reference to a Channel object that I didn't have anymore. I think after a Channel is properly closed, these callbacks should be removed. I think currently, the `_on_basic_deliver` callback is still routing those messages into the `self._pending` dict which is now safer, but doing it for more than one Channel!

We can't quite use git head yet as there is a separate issue there in that the aforementioned commit appears to make everything unbearably slow so we're stuck on 0.9.5 for the time being. My solution was to, in our registered callback for a successful Channel close, is to poke around in the callback manager like such:

```python
self.amq_chan.callbacks.remove(self.amq_chan.channel_number, '_on_basic_deliver')
self.amq_chan.callbacks.remove(self.amq_chan.channel_number, '_on_basic_get')
```

This is working for now, but is clearly not optimal.