leporo / tornado-redis

Asynchronous Redis client that works within Tornado IO loop.
666 stars 162 forks source link

unsubscribe callback not called when listen loop not running #81

Open fpn opened 9 years ago

fpn commented 9 years ago

We fell into a trap when using the unsubscribe, the callback does not trigger when the subscription listen loop has been exited or is not running yet.

So if an app does:

@gen.coroutine
def cleanup(self):
    yield gen.Task(self.client.unsubscribe, channels=self.subscriptions)
    yield gen.Task(self.client.disconnect)

There is a good chance that in the real world disconnect is not called.

I suggest you make unsubscribe an idempotent operation and always fire the callback, even if there is no subscription or not subscription loop running.

Test cases below:

    @tornado.testing.gen_test(timeout=60)
    def test_rainy_day(self):
        subscriptions = ["one", "two"]
        self.client = tornadoredis.Client(host=options.redis_host)
        self.client.connect()
        result = yield gen.Task(self.client.execute_command, 'CLIENT', 'SETNAME', 'foo')
        log.debug(result)
        yield gen.Task(self.client.subscribe, subscriptions)
        self.client.listen(self.on_redis_msg, exit_callback=self.on_redis_pub_sub_exit)
        log.debug("subscribed to: %s", subscriptions)
        yield gen.Task(self.client.unsubscribe, channels=subscriptions)
        log.debug("unsub 1")
        yield gen.Task(self.client.unsubscribe, channels=subscriptions)
        log.debug("unsub 2")
        result = yield gen.Task(self.client.disconnect)

    @tornado.testing.gen_test(timeout=60)
    def test_rainy_day_two(self):
        subscriptions = ["one", "two"]
        self.client = tornadoredis.Client(host=options.redis_host)
        self.client.connect()
        result = yield gen.Task(self.client.execute_command, 'CLIENT', 'SETNAME', 'foo')
        log.debug(result)
        yield gen.Task(self.client.unsubscribe, channels=subscriptions)
        log.debug("unsub 2")
        result = yield gen.Task(self.client.disconnect)