Repository: django/channels (python)

Issue:
InMemoryChannelLayer grows indefinitely
InMemoryChannelLayer has this in the `_clean_expired` method:

            # Is the channel now empty and needs deleting?
            if not queue:
                del self.channels[channel]

Queues are always truthy even if empty. So that `del` statement will not be reached. I believe the line should be changed to `if queue.empty():`. 

(I saw this while investigating a crash that seems to be due to the channel layer growing indefinitely bigger.)

Pull request description:
Fix InMemoryChannelLayer growing indefinitely
In `InMemoryChannelLayer` it was possible for channels with expired messages to live indefinitely since the check for deleting them always evaluated to `False`.

Fixes #1574.

Patch:
diff --git a/channels/layers.py b/channels/layers.py
index fa9a9aa..5f26037 100644
--- a/channels/layers.py
+++ b/channels/layers.py
@@ -247,11 +247,11 @@ class InMemoryChannelLayer(BaseChannelLayer):
         queue = self.channels.setdefault(channel, asyncio.Queue())
 
         # Do a plain direct receive
-        _, message = await queue.get()
-
-        # Delete if empty
-        if queue.empty():
-            del self.channels[channel]
+        try:
+            _, message = await queue.get()
+        finally:
+            if queue.empty():
+                del self.channels[channel]
 
         return message
 
@@ -274,17 +274,14 @@ class InMemoryChannelLayer(BaseChannelLayer):
         """
         # Channel cleanup
         for channel, queue in list(self.channels.items()):
-            remove = False
             # See if it's expired
             while not queue.empty() and queue._queue[0][0] < time.time():
                 queue.get_nowait()
-                remove = True
-            # Any removal prompts group discard
-            if remove:
+                # Any removal prompts group discard
                 self._remove_from_groups(channel)
-            # Is the channel now empty and needs deleting?
-            if not queue:
-                del self.channels[channel]
+                # Is the channel now empty and needs deleting?
+                if queue.empty():
+                    del self.channels[channel]
 
         # Group Expiration
         timeout = int(time.time()) - self.group_expiry


Test patch:
diff --git a/tests/test_inmemorychannel.py b/tests/test_inmemorychannel.py
index 6544f7a..90e02b1 100644
--- a/tests/test_inmemorychannel.py
+++ b/tests/test_inmemorychannel.py
@@ -106,3 +106,65 @@ async def test_groups_channel_full(channel_layer):
     await channel_layer.group_send("test-group", {"type": "message.1"})
     await channel_layer.group_send("test-group", {"type": "message.1"})
     await channel_layer.group_send("test-group", {"type": "message.1"})
+
+
+@pytest.mark.asyncio
+async def test_expiry_single():
+    """
+    Tests that a message can expire.
+    """
+    channel_layer = InMemoryChannelLayer(expiry=0.1)
+    await channel_layer.send("test-channel-1", {"type": "message.1"})
+    assert len(channel_layer.channels) == 1
+
+    await asyncio.sleep(0.1)
+
+    # Message should have expired and been dropped.
+    with pytest.raises(asyncio.TimeoutError):
+        async with async_timeout.timeout(0.5):
+            await channel_layer.receive("test-channel-1")
+
+    # Channel should be cleaned up.
+    assert len(channel_layer.channels) == 0
+
+
+@pytest.mark.asyncio
+async def test_expiry_unread():
+    """
+    Tests that a message on a channel can expire and be cleaned up even if
+    the channel is not read from again.
+    """
+    channel_layer = InMemoryChannelLayer(expiry=0.1)
+    await channel_layer.send("test-channel-1", {"type": "message.1"})
+
+    await asyncio.sleep(0.1)
+
+    await channel_layer.send("test-channel-2", {"type": "message.2"})
+    assert len(channel_layer.channels) == 2
+    assert (await channel_layer.receive("test-channel-2"))["type"] == "message.2"
+    # Both channels should be cleaned up.
+    assert len(channel_layer.channels) == 0
+
+
+@pytest.mark.asyncio
+async def test_expiry_multi():
+    """
+    Tests that multiple messages can expire.
+    """
+    channel_layer = InMemoryChannelLayer(expiry=0.1)
+    await channel_layer.send("test-channel-1", {"type": "message.1"})
+    await channel_layer.send("test-channel-1", {"type": "message.2"})
+    await channel_layer.send("test-channel-1", {"type": "message.3"})
+    assert (await channel_layer.receive("test-channel-1"))["type"] == "message.1"
+
+    await asyncio.sleep(0.1)
+    await channel_layer.send("test-channel-1", {"type": "message.4"})
+    assert (await channel_layer.receive("test-channel-1"))["type"] == "message.4"
+
+    # The second and third message should have expired and been dropped.
+    with pytest.raises(asyncio.TimeoutError):
+        async with async_timeout.timeout(0.5):
+            await channel_layer.receive("test-channel-1")
+
+    # Channel should be cleaned up.
+    assert len(channel_layer.channels) == 0