Skip to content

Fix resource leak in WebSocket handler by properly awaiting cancelled tasks #253

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yugalkaushik wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix resource leak in WebSocket handler by properly awaiting cancelled tasks #253

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Backend/app/routes/chat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,18 @@ async def websocket_endpoint(

except WebSocketDisconnect:
listener_task.cancel()
try:
await listener_task
except asyncio.CancelledError:
pass
await chat_service.disconnect(user_id, redis, db)

except Exception as e:
listener_task.cancel()
try:
await listener_task
except asyncio.CancelledError:
pass
await chat_service.disconnect(user_id, redis, db)
Comment on lines 51 to 57
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's look at the file structure
find Backend -name "chat.py" -type f | head -20

Repository: AOSSIE-Org/InPactAI

Length of output: 116

🏁 Script executed:

#!/bin/bash
# Look at the specific lines mentioned (51-57) with context
if [ -f "Backend/app/routes/chat.py" ]; then
    echo "=== Lines 40-70 of Backend/app/routes/chat.py ==="
    sed -n '40,70p' Backend/app/routes/chat.py | cat -n
fi

Repository: AOSSIE-Org/InPactAI

Length of output: 1250

🏁 Script executed:

#!/bin/bash
# Search for listen_to_channel function
rg -n "listen_to_channel" Backend/app --type py -A 5 -B 2

Repository: AOSSIE-Org/InPactAI

Length of output: 1558

🏁 Script executed:

#!/bin/bash
# Get the full listen_to_channel function implementation
rg -n "async def listen_to_channel" Backend/app/services/chat_pubsub.py -A 30

Repository: AOSSIE-Org/InPactAI

Length of output: 530

🏁 Script executed:

#!/bin/bash
# Also check the full file to understand structure better
wc -l Backend/app/services/chat_pubsub.py

Repository: AOSSIE-Org/InPactAI

Length of output: 101

🏁 Script executed:

#!/bin/bash
# View the full Backend/app/services/chat_pubsub.py file if it's not too large
cat -n Backend/app/services/chat_pubsub.py

Repository: AOSSIE-Org/InPactAI

Length of output: 689

🏁 Script executed:

#!/bin/bash
# Check what chat_service.disconnect does
rg -n "def disconnect" Backend/app/services --type py -A 10

Repository: AOSSIE-Org/InPactAI

Length of output: 959

Task cancellation and pubsub cleanup are correct.

The exception handler properly cancels the listener task and awaits it with CancelledError suppression. The listen_to_channel function includes a finally block that unsubscribes and closes the Redis pubsub connection, so cleanup will execute when the task is cancelled.

However, two improvements are needed:

  1. Explicitly close the websocket after an exception: Currently, chat_service.disconnect() only removes the connection from tracking and updates user status—it does not explicitly close the websocket. After a generic exception, the websocket may be left in an inconsistent state. Consider calling await websocket.close() before cleanup.

  2. Replace print() with proper logging: Use a logger instead of print() for production-ready error reporting.

🧰 Tools
🪛 Ruff (0.14.10)

51-51: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents 
In Backend/app/routes/chat.py around lines 51 to 57, update the exception
handler to (1) explicitly close the websocket before doing service cleanup by
awaiting websocket.close() so the socket is left in a consistent state, and (2)
replace any print() calls with the module logger (e.g. logger.error(...)) and
include exception information (the exception object/traceback) in the log
message; then proceed to cancel/await the listener task and call
chat_service.disconnect(user_id, redis, db) as before.

# Optionally log the error
print(f"Error in websocket for user {user_id}: {e}")
Expand Down