Skip to content

Fix MongoDB connection leak with singleton pattern and auto-reconnection#136

Open
Copilot wants to merge 8 commits intomainfrom
copilot/fix-mongodb-connection-leak
Open

Fix MongoDB connection leak with singleton pattern and auto-reconnection#136
Copilot wants to merge 8 commits intomainfrom
copilot/fix-mongodb-connection-leak

Conversation

Copy link
Contributor

Copilot AI commented Jan 29, 2026

MongoDB Connection Leak Fix - Complete ✅

Analysis Complete

  • Identify MongoDB connection handling in codebase
  • Understand current issue: dbConnect() creates new clients without closing them

Implementation Complete

  • Implement singleton pattern for MongoDB client
  • Modify dbConnect() to reuse a single client connection
  • Add proper connection lifecycle management
  • Build succeeds without errors
  • Address all code review feedback
  • Implement lazy connection initialization
  • Security scan passed (0 vulnerabilities)
  • Clean up connection promise after fulfillment
  • Add error handling to connection close
  • Refactor to return db from connectDatabase (cleaner code)
  • Add automatic reconnection on disconnect detection
  • Add error handling for connection failures
  • Add shutdown timeout to prevent hanging

Final Solution

db/utils.ts - Connection Management:

- Singleton pattern with module-level client variable (removed global db)
- connectionPromise prevents race conditions during initialization
- connectDatabase(): Returns Db instance, initializes once, reuses promise if already connecting
- Connection failure handling: Cleans up client on failure to prevent inconsistent state
- Auto-detects disconnections via 'close' event and clears client for automatic reconnection
- Event listener cleanup: Removes all listeners before closing connection
- closeDatabaseConnection(): Try-catch for error handling, always resets state in finally block
- dbConnect(): Gets db from connectDatabase(), no type assertions needed

index.ts - Application Lifecycle:

- Removed eager connection initialization
- Database connects lazily on first request/cron job
- Graceful shutdown handlers (SIGTERM, SIGINT):
  - 30-second timeout to prevent hanging on long-lived connections
  - Awaits server.close() completion or timeout
  - Closes database connection
  - Clean process exit

Benefits Delivered

No Connection Leaks - Single connection reused across all requests
Lazy Initialization - Database connects only when needed
Better Error Handling - Server starts even if MongoDB temporarily unavailable
Connection Failure Recovery - Cleans up failed connections properly
Race Condition Prevention - Concurrent connection attempts use same promise
Graceful Shutdown - Properly closes server and database connections with timeout
Robust Cleanup - Connection promise cleaned up after use, error handling on close
Cleaner Code - One less global variable, no type assertions needed
Auto-Reconnection - Detects disconnections and automatically reconnects on next use
Memory Leak Prevention - Event listeners properly cleaned up
Security - No vulnerabilities introduced

Recent Updates

  • Added try-catch error handling in connection promise
  • Clean up client reference on connection failure
  • Added 30-second timeout to graceful shutdown
  • Prevents shutdown from hanging on long-lived connections

Technical Details

  • Before: Each dbConnect() created new MongoClient, never closed → connection leaks
  • After: Single MongoClient created on first use, reused for all subsequent requests
  • Connection Failure: Client is cleaned up on failure, next call attempts fresh connection
  • Disconnect Handling: Client monitors connection state and auto-clears on disconnect for automatic recovery
  • Shutdown Timeout: Uses Promise.race to ensure graceful shutdown completes within 30 seconds
  • MongoDB driver's built-in connection pooling handles concurrent requests efficiently

Testing Status

  • TypeScript compilation successful
  • Security scan passed
  • Manual testing recommended to verify behavior in production environment
Original prompt

This section details on the original issue you should resolve

<issue_title>Connections to MongoDB are not closed properly</issue_title>
<issue_description>The Nodejs application is opening a new connection for each query it seems but do not close this correctly, so the server leaks connections</issue_description>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits January 29, 2026 07:55
- Implement singleton pattern for MongoDB client
- Reuse single connection across all requests
- Add proper connection lifecycle management
- Initialize connection on startup
- Close connection gracefully on shutdown

Co-authored-by: Apollon77 <11976694+Apollon77@users.noreply.github.com>
- Fix race condition in connectDatabase() with connection promise
- Implement lazy connection initialization on first use
- Fix graceful shutdown to properly await server.close()
- Remove eager connection on startup for better error handling

Co-authored-by: Apollon77 <11976694+Apollon77@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix improper closure of MongoDB connections Fix MongoDB connection leak with singleton pattern and lazy initialization Jan 29, 2026
Copilot AI requested a review from Apollon77 January 29, 2026 08:04
- Clean up connectionPromise after it's fulfilled in connectDatabase()
- Add try-catch error handling to closeDatabaseConnection()
- Ensure state is always reset even if close() throws an error

Co-authored-by: Apollon77 <11976694+Apollon77@users.noreply.github.com>
Copy link
Collaborator

@Apollon77 Apollon77 left a comment

Choose a reason for hiding this comment

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

@copilot what about returning "db" when using connect then the type tricks and such are not needed and we use one global variable less in all code?

- Change connectDatabase() to return Db instead of Promise<void>
- Remove global db variable, use only client
- Change connectionPromise to Promise<Db>
- Eliminate need for type assertions (db!) in dbConnect()
- Simplify code with one less global variable

Co-authored-by: Apollon77 <11976694+Apollon77@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 29, 2026

@copilot what about returning "db" when using connect then the type tricks and such are not needed and we use one global variable less in all code?

Implemented in commit 18f3e6c. The connectDatabase() now returns Db, eliminating the global db variable and removing the need for type assertions. This simplifies the code with one less global variable.

- Listen for 'close' event on MongoDB client
- Automatically clear client and connectionPromise on disconnect
- Next dbConnect() call will create new connection automatically
- Also add 'error' event listener for better error visibility

Co-authored-by: Apollon77 <11976694+Apollon77@users.noreply.github.com>
Copilot AI changed the title Fix MongoDB connection leak with singleton pattern and lazy initialization Fix MongoDB connection leak with singleton pattern and auto-reconnection Jan 29, 2026
Copilot AI requested a review from Apollon77 January 29, 2026 08:38
@Apollon77 Apollon77 marked this pull request as ready for review January 29, 2026 09:37
Copilot AI review requested due to automatic review settings January 29, 2026 09:37
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes MongoDB connection leaks by implementing a singleton pattern for database connections, adds graceful shutdown handlers, and introduces automatic reconnection on disconnect detection.

Changes:

  • Implemented singleton pattern in db/utils.ts to reuse a single MongoDB client across all requests instead of creating new clients for each query
  • Added graceful shutdown handlers in index.ts for SIGTERM and SIGINT signals to properly close HTTP server and database connections
  • Added auto-reconnection logic with MongoDB event listeners to detect and recover from unexpected disconnections

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
express/backend/src/db/utils.ts Implements singleton pattern with module-level client variable, connection promise for race condition prevention, auto-reconnection via event listeners, and proper cleanup functions
express/backend/src/index.ts Adds graceful shutdown handlers for SIGTERM/SIGINT signals that close the HTTP server and database connection before exiting

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 28 to 43
await client.connect();

// Set up event listeners to detect disconnections
client.on("close", () => {
console.log("MongoDB connection closed unexpectedly");
client = null;
connectionPromise = null;
});

client.on("error", (error) => {
console.error("MongoDB client error:", error);
});

const db = client.db();
console.log("MongoDB connected successfully");
return db;
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

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

If the connection attempt fails (e.g., MongoDB is unreachable), the error will be thrown and propagated to the caller, but the client variable may be left in an inconsistent state. The client will be set to a new MongoClient instance at line 27, but if client.connect() fails at line 28, the client variable will still reference the failed client object. Subsequent calls to connectDatabase() will then pass the check at line 17 and return client.db() for a client that never successfully connected, which will likely fail.

Add error handling in the connection promise to ensure that if the connection fails, the client variable is reset to null so that the next call to connectDatabase() will attempt a fresh connection.

Suggested change
await client.connect();
// Set up event listeners to detect disconnections
client.on("close", () => {
console.log("MongoDB connection closed unexpectedly");
client = null;
connectionPromise = null;
});
client.on("error", (error) => {
console.error("MongoDB client error:", error);
});
const db = client.db();
console.log("MongoDB connected successfully");
return db;
try {
await client.connect();
// Set up event listeners to detect disconnections
client.on("close", () => {
console.log("MongoDB connection closed unexpectedly");
client = null;
connectionPromise = null;
});
client.on("error", (error) => {
console.error("MongoDB client error:", error);
});
const db = client.db();
console.log("MongoDB connected successfully");
return db;
} catch (error) {
console.error("Error connecting to MongoDB:", error);
if (client) {
try {
await client.close();
} catch (closeError) {
console.error("Error closing MongoDB client after failed connection:", closeError);
}
}
client = null;
throw error;
}

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@copilot Please verify this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Verified and fixed in commit 55a4a43. Added try-catch around client.connect() to handle connection failures. If connection fails, the client is properly cleaned up (closed and set to null) to prevent inconsistent state. The error is then re-thrown to propagate to the caller.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Wrap client.connect() in try-catch to handle connection failures
- Clean up client on connection failure to prevent inconsistent state
- Add 30-second timeout to graceful shutdown to prevent hanging
- Use Promise.race to ensure shutdown completes within timeout

Co-authored-by: Apollon77 <11976694+Apollon77@users.noreply.github.com>
Copy link
Collaborator

@UncleSamSwiss UncleSamSwiss left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, please find my comments below. Feel free to change things if you agree, or leave it as-is.

let client: MongoClient | null = null;
let connectionPromise: Promise<Db> | null = null;

export function createClient() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

As nobody should create DB clients directly, we shouldn't export this function.

return new MongoClient("mongodb://mongo/dev-portal");
}

export async function connectDatabase(): Promise<Db> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We might want to remove the export here because everybody should use dbConnect() instead. But leaving this function exported doesn't hurt either.

startCronJobs();

// Graceful shutdown helper
async function gracefulShutdown(signal: string) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if we really need graceful shutdown handling as none of these resources are persistent beyond the process lifetime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Connections to MongoDB are not closed properly

4 participants