Эх сурвалжийг харах

fix: resolve memory leak issues causing OOM #41

Fixed 5 critical memory leak issues:

1. jobQueue.js - Added periodic cleanup timer for recentlyProcessed Map
   - Timer runs every 10 minutes to remove old entries
   - Map is cleared on shutdown to prevent unbounded growth

2. emailNotifier.js - Implemented LRU cache for userEmails Map
   - Cache limited to 1000 entries maximum
   - Oldest entry removed when cache is full
   - Cache cleared on graceful shutdown

3. logger.js - Fixed metrics timer leak
   - Added shutdown() method to clear timer
   - File stream properly closed on shutdown
   - Prevents Logger instance from staying in memory

4. server.js - Added request body size limit
   - Default max size: 10MB
   - Protects against large payload attacks
   - Request destroyed if limit exceeded

5. index.js - Improved graceful shutdown
   - All cleanup methods called on SIGTERM/SIGINT
   - Ensures proper resource cleanup

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Claude 8 сар өмнө
parent
commit
b5530ad81e
5 өөрчлөгдсөн 87 нэмэгдсэн , 8 устгасан
  1. 14 0
      src/emailNotifier.js
  2. 4 0
      src/index.js
  3. 35 7
      src/jobQueue.js
  4. 22 1
      src/logger.js
  5. 12 0
      src/server.js

+ 14 - 0
src/emailNotifier.js

@@ -18,6 +18,7 @@ export class EmailNotifier {
     this.sender = null;
     this.rules = [];
     this.userEmails = new Map(); // Cache for username -> email mapping
+    this.maxCacheSize = 1000; // Maximum number of cached email addresses
     this.loadConfig();
   }
 
@@ -325,6 +326,11 @@ export class EmailNotifier {
     for (const user of users) {
       if (user.username === username || user.login === username) {
         if (user.email) {
+          // Implement LRU-like behavior: if cache is full, remove oldest entry
+          if (this.userEmails.size >= this.maxCacheSize) {
+            const firstKey = this.userEmails.keys().next().value;
+            this.userEmails.delete(firstKey);
+          }
           this.userEmails.set(username, user.email);
           return user.email;
         }
@@ -335,6 +341,14 @@ export class EmailNotifier {
     return null;
   }
 
+  /**
+   * Clear email cache (for memory management)
+   */
+  clearEmailCache() {
+    this.userEmails.clear();
+    logger.debug('Email cache cleared');
+  }
+
   /**
    * Substitute variables in template strings
    */

+ 4 - 0
src/index.js

@@ -75,13 +75,17 @@ logger.info('Job queue initialized', queueStats);
 process.on('SIGTERM', async () => {
   logger.info('SIGTERM received, shutting down gracefully');
   await jobQueue.shutdown();
+  emailNotifier.clearEmailCache();
   await server.stop();
+  logger.shutdown();
   process.exit(0);
 });
 
 process.on('SIGINT', async () => {
   logger.info('SIGINT received, shutting down gracefully');
   await jobQueue.shutdown();
+  emailNotifier.clearEmailCache();
   await server.stop();
+  logger.shutdown();
   process.exit(0);
 });

+ 35 - 7
src/jobQueue.js

@@ -32,6 +32,11 @@ export class JobQueue {
     // Track recently processed issue events to prevent duplicates
     this.recentlyProcessed = new Map(); // key: "issues-15", value: timestamp
 
+    // Start periodic cleanup of old deduplication entries
+    this.cleanupTimer = setInterval(() => {
+      this._cleanupRecentlyProcessed();
+    }, 600000); // Run cleanup every 10 minutes
+
     // Load existing queue from file
     this.loadQueue();
   }
@@ -163,6 +168,27 @@ export class JobQueue {
     return this.queues.get(repoName);
   }
 
+  /**
+   * Clean up old entries from recentlyProcessed Map
+   * This prevents unbounded memory growth
+   */
+  _cleanupRecentlyProcessed() {
+    const now = Date.now();
+    let removedCount = 0;
+
+    for (const [key, timestamp] of this.recentlyProcessed.entries()) {
+      // Remove entries older than 10 minutes
+      if (now - timestamp > 600000) {
+        this.recentlyProcessed.delete(key);
+        removedCount++;
+      }
+    }
+
+    if (removedCount > 0) {
+      logger.deduplicationCleanup(removedCount);
+    }
+  }
+
   /**
    * Add a job to the queue
    * @param {string} eventType - The webhook event type
@@ -192,13 +218,6 @@ export class JobQueue {
         }
 
         this.recentlyProcessed.set(key, now);
-
-        // Cleanup old entries (older than 10 minutes)
-        for (const [k, ts] of this.recentlyProcessed.entries()) {
-          if (now - ts > 600000) {
-            this.recentlyProcessed.delete(k);
-          }
-        }
       }
     }
 
@@ -551,6 +570,15 @@ export class JobQueue {
    * Graceful shutdown - save queue state
    */
   async shutdown() {
+    // Clear cleanup timer to prevent memory leak
+    if (this.cleanupTimer) {
+      clearInterval(this.cleanupTimer);
+      this.cleanupTimer = null;
+    }
+
+    // Clear recentlyProcessed Map to free memory
+    this.recentlyProcessed.clear();
+
     if (this.parallelPerRepository) {
       // Multi-queue mode: save all queues
       let totalJobs = 0;

+ 22 - 1
src/logger.js

@@ -57,6 +57,7 @@ export class Logger {
     };
 
     // Reset metrics every 24 hours
+    this.metricsTimer = null;
     this._startMetricsReset();
   }
 
@@ -169,7 +170,7 @@ export class Logger {
 
   _startMetricsReset() {
     // Reset metrics every 24 hours
-    setInterval(() => {
+    this.metricsTimer = setInterval(() => {
       this.metrics = {
         jobsProcessed24h: 0,
         jobsFailed24h: 0,
@@ -181,6 +182,26 @@ export class Logger {
     }, 24 * 60 * 60 * 1000);
   }
 
+  /**
+   * Shutdown logger and cleanup resources
+   */
+  shutdown() {
+    // Clear metrics timer to prevent memory leak
+    if (this.metricsTimer) {
+      clearInterval(this.metricsTimer);
+      this.metricsTimer = null;
+    }
+
+    // Close file stream if open
+    if (this.fileStream) {
+      this.fileStream.end();
+      this.fileStream = null;
+    }
+
+    // Clear metrics to free memory
+    this.metrics.processingTimes = [];
+  }
+
   _getTimestamp() {
     return this.enableTimestamps ? `[${new Date().toISOString()}]` : '';
   }

+ 12 - 0
src/server.js

@@ -12,6 +12,7 @@ export class WebhookServer {
     this.port = options.port || 3000;
     this.path = options.path || '/webhook';
     this.secret = options.secret || null;
+    this.maxBodySize = options.maxBodySize || 10 * 1024 * 1024; // 10MB default
     this.webhookHandler = new WebhookHandler();
     this.server = null;
   }
@@ -23,7 +24,18 @@ export class WebhookServer {
   _parseBody(req) {
     return new Promise((resolve, reject) => {
       let body = '';
+      let bodySize = 0;
+
       req.on('data', chunk => {
+        bodySize += chunk.length;
+
+        // Protect against large payloads that could cause OOM
+        if (bodySize > this.maxBodySize) {
+          req.destroy();
+          reject(new Error(`Request body too large (max: ${this.maxBodySize} bytes)`));
+          return;
+        }
+
         body += chunk.toString();
       });
       req.on('end', () => {