# Tracing a memory leak in Solid Queue workers

## Memory Leak Analysis - Stimulus Controllers

## Summary
Analysis of memory leak issues in Stimulus controllers, particularly related to event listener cleanup and timer management when navigating with Turbo.

---

## 🔴 Critical Issues Found

### 1. **password_validator_controller.js** - Event Listener Memory Leak
**Status:** ❌ **CONFIRMED MEMORY LEAK**

**Location:** Lines 49-60 (disconnect), 62-70 (setupPasswordToggle), 72-81 (setupFormSubmission)

**Problem:**
- Event listeners are added with `.bind(this)` in `connect()` and `setupPasswordToggle()`
- In `disconnect()`, new `.bind(this)` references are created, which don't match the original listeners
- Result: Event listeners are never removed, causing memory leaks on Turbo navigation

**Current Code:**
```javascript
// connect() calls setupPasswordToggle() and setupFormSubmission()
setupPasswordToggle() {
  this.toggleButtonTarget.addEventListener('click', this.togglePasswordVisibility.bind(this))
}

setupFormSubmission() {
  form.addEventListener('submit', this.handleFormSubmit.bind(this))
}

disconnect() {
  // ❌ These won't work - new function references!
  this.toggleButtonTarget.removeEventListener('click', this.togglePasswordVisibility.bind(this))
  form.removeEventListener('submit', this.handleFormSubmit.bind(this))
}
```

**Impact:** 
- Memory leaks accumulate on every Turbo navigation
- Event listeners fire multiple times (one per navigation)
- Performance degradation over time

---

### 2. **confetti_controller.js** - setInterval Not Cleared
**Status:** ⚠️ **POTENTIAL MEMORY LEAK**

**Location:** Line 50 (setInterval)

**Problem:**
- `setInterval` is created in `fire()` method
- Interval is only cleared inside its own callback (when timeLeft <= 0)
- If controller disconnects before interval completes, interval continues running
- No reference stored to clear it in `disconnect()`

**Current Code:**
```javascript
connect() {
  setTimeout(() => {
    this.fire()  // Creates setInterval
  }, 300)
}

fire() {
  const interval = setInterval(() => {
    // ... confetti logic
    if (timeLeft <= 0) {
      return clearInterval(interval)  // Only cleared here
    }
  }, 250)
  // ❌ No reference stored, can't clear in disconnect()
}
```

**Impact:**
- Interval continues running after controller disconnects
- Memory leak if user navigates away quickly
- Unnecessary CPU usage

---

### 3. **rate_limit_controller.js** - setTimeout Not Cleared
**Status:** ⚠️ **POTENTIAL MEMORY LEAK**

**Location:** Line 125 (setTimeout in scheduleReEnable)

**Problem:**
- `setTimeout` is created but never stored
- If controller disconnects before timeout fires, it still executes
- Could cause errors if controller is disconnected

**Current Code:**
```javascript
scheduleReEnable() {
  setTimeout(() => {
    this.enableForm()  // ❌ May execute after disconnect
    // ...
  }, this.retryAfterValue * 1000)
  // No reference stored to clear it
}
```

**Impact:**
- Timeout may execute after controller disconnects
- Potential errors accessing `this.enableForm()` on disconnected controller
- Memory leak (timeout reference not cleared)

---

## ✅ Correctly Implemented Controllers

### **autosave_controller.js** - ✅ Properly Handled
- Stores bound handlers: `this.boundScheduleAutosave`, `this.lexxyChangeHandler`, etc.
- Properly removes listeners in `disconnect()`
- Clears timeouts correctly

### **modal_controller.js** - ✅ Properly Handled
- Stores bound handler: `this.boundAnchorHandler`
- Properly removes listener in `disconnect()`

---

## 📋 Controllers Without Issues

These controllers don't add event listeners or timers:
- `link_unfurl_controller.js` - Uses data-action attributes
- `unsplash_picker_controller.js` - Uses data-action attributes
- `flash_injector_controller.js` - Only DOM manipulation in connect()
- `tabs_controller.js` - Only Bootstrap initialization
- `syntax_highlight_controller.js` - Only DOM manipulation

---

## 🔧 Recommended Fixes

### Fix 1: password_validator_controller.js
Store bound handlers in `connect()` and use them in `disconnect()`:

```javascript
connect() {
  // Store bound handlers
  this.boundTogglePasswordVisibility = this.togglePasswordVisibility.bind(this)
  this.boundHandleFormSubmit = this.handleFormSubmit.bind(this)
  
  // ... rest of connect logic
}

setupPasswordToggle() {
  if (this.hasToggleButtonTarget) {
    this.toggleButtonTarget.addEventListener('click', this.boundTogglePasswordVisibility)
  }
}

setupFormSubmission() {
  const form = this.element.querySelector('form')
  if (form) {
    form.addEventListener('submit', this.boundHandleFormSubmit)
  }
}

disconnect() {
  if (this.hasToggleButtonTarget && this.boundTogglePasswordVisibility) {
    this.toggleButtonTarget.removeEventListener('click', this.boundTogglePasswordVisibility)
  }
  
  const form = this.element.querySelector('form')
  if (form && this.boundHandleFormSubmit) {
    form.removeEventListener('submit', this.boundHandleFormSubmit)
  }
  
  this.isSubmitting = false
}
```

### Fix 2: confetti_controller.js
Store interval reference and clear in disconnect:

```javascript
connect() {
  this.confettiTimeout = setTimeout(() => {
    this.fire()
  }, 300)
}

disconnect() {
  if (this.confettiTimeout) {
    clearTimeout(this.confettiTimeout)
    this.confettiTimeout = null
  }
  if (this.confettiInterval) {
    clearInterval(this.confettiInterval)
    this.confettiInterval = null
  }
}

fire() {
  // ... existing code ...
  this.confettiInterval = setInterval(() => {
    // ... existing logic ...
  }, 250)
}
```

### Fix 3: rate_limit_controller.js
Store timeout reference, clear in disconnect, and add disconnection flag to prevent closure memory leaks:

```javascript
connect() {
  // Reset disconnection flag when controller connects
  this.isDisconnected = false
  
  // ... rest of connect logic
}

disconnect() {
  if (this.reEnableTimeout) {
    clearTimeout(this.reEnableTimeout)
    this.reEnableTimeout = null
  }
  // Mark as disconnected for any pending callbacks to prevent memory leaks
  // Even if clearTimeout is called, the closure may still hold a reference
  this.isDisconnected = true
}

scheduleReEnable() {
  if (this.retryAfterValue > 0) {
    this.reEnableTimeout = setTimeout(() => {
      // Double-check disconnection state to prevent memory leaks
      // Even if clearTimeout is called, the closure may still hold a reference
      if (this.isDisconnected || !this.element || !this.hasErrorContainerTarget) {
        return
      }
      
      this.enableForm()
      // ... rest of logic ...
      
      this.reEnableTimeout = null
    }, this.retryAfterValue * 1000)
  }
}
```

**Why the `isDisconnected` flag?**
- Even after `clearTimeout()` is called, the closure created by the arrow function still holds a reference to `this` (the controller instance)
- In race conditions where `disconnect()` is called but the timeout callback hasn't been garbage collected yet, the closure prevents the controller from being fully released
- The `isDisconnected` flag ensures the callback exits early and doesn't access controller properties, allowing garbage collection

---

## 📊 Impact Assessment

| Controller | Severity | Frequency | Impact |
|------------|----------|-----------|--------|
| password_validator_controller.js | 🔴 High | Every navigation | Memory leak, duplicate listeners |
| confetti_controller.js | 🟡 Medium | On navigation during animation | Memory leak, CPU waste |
| rate_limit_controller.js | 🟡 Medium | On navigation during rate limit | Potential errors, memory leak |

---

## ✅ Verification Checklist

After fixes:
- [ ] All event listeners use stored bound handlers
- [ ] All `removeEventListener` calls use the same reference as `addEventListener`
- [ ] All `setTimeout`/`setInterval` references are stored
- [ ] All timers are cleared in `disconnect()`
- [ ] Disconnection flags added for timeout callbacks to prevent closure memory leaks
- [ ] Test with Turbo navigation to verify cleanup